fix(alignment): apply alignment to all blocks in a multi-block selection#191
Merged
Conversation
setAlignment resolved only the anchor block, so choosing an alignment with several blocks selected aligned just the first one and silently ignored the rest. TextDirectionPlugin already handled the full range, confirming this was incomplete behavior. Extract the range-resolution logic into a shared getSelectedBlockIds helper in PluginHelpers, consume it from both AlignmentPlugin and TextDirectionPlugin (removing the latter's private duplicate), and rewrite setAlignment to apply the align attribute to every alignable block in the selection in a single transaction. Sharing one code path keeps alignment and direction from diverging again. Closes #190
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #190.
Choosing a text alignment with several blocks selected aligned only the anchor block and silently ignored the rest.
TextDirectionPluginalready handled the full range correctly, which confirmed this was incomplete behavior rather than a design choice.Root cause
AlignmentPlugin.setAlignmentresolved a single block (getSelectedBlock+getSelectedBlockId) and appliedalignonly to that anchor.Fix
Rather than duplicating text-direction's range logic into alignment (which would let the two diverge again), the shared logic is extracted so both plugins run one code path:
PluginHelpers.ts— new exportedgetSelectedBlockIds(state): returns every leaf-block ID covered by the selection (NodeSelection → its node; single/collapsed → one ID; multi-block → the full inclusive anchor→head range in document order).TextDirectionPlugin.ts— deletes its private duplicate and consumes the shared helper.AlignmentPlugin.ts—setAlignmentnow iterates every selected block, appliesalignto each alignable one (skipping non-alignable blocks), and commits a single transaction.Toolbar
isActive/isEnabledare intentionally left checking the anchor only, matching text-direction's existing behavior.Tests
Two multi-block cases added to
AlignmentPlugin.test.ts, mirroring the text-direction suite:continuebranch)Verification
text-direction.spec.ts(3 tests) passes, confirming the extraction is behavior-preserving in a real browserNote (out of scope)
Alignment does not
announce()to screen readers on any path, whereas text-direction does. This is a pre-existing gap orthogonal to this bug (no a11y regression introduced); worth a separate follow-up ticket.