Fix increaseNestingLevel/decreaseNestingLevel for multi-item list selections#1319
Open
blshkv wants to merge 2 commits into
Open
Fix increaseNestingLevel/decreaseNestingLevel for multi-item list selections#1319blshkv wants to merge 2 commits into
blshkv wants to merge 2 commits into
Conversation
Both methods called getBlock() which returns only the first block in the selection (locationRange[0].index). When multiple list items were selected, only the first item's nesting level changed. Fix: iterate over every block index from locationRange[0].index to locationRange[1].index, chaining replaceBlock() calls through the updated document so all selected items are adjusted atomically before setDocument() is called. Add regression tests that select three items and assert all three change level.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extend list nesting (indent/outdent) actions to operate over the current multi-block selection instead of only the active block, and add system tests to validate the selection-wide behavior.
Changes:
- Update
increaseNestingLevel/decreaseNestingLevelto iterate over the selection’s index range and apply the nesting transform per block. - Add system tests asserting nesting changes apply across multiple selected list items.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/trix/models/composition.js | Applies nesting level changes across the selected range instead of a single block. |
| src/test/system/list_formatting_test.js | Adds system tests for selection-wide increase/decrease nesting actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Skip the end block when selection ends at offset 0 (caret at block start means that block is not covered by the selection). Applied to both increaseNestingLevel and decreaseNestingLevel. - Fix decrease-nesting test: item c starts at level 2, one decreaseNestingLevel call brings it to level 1, not level 0. Corrected expected blockAttributes.
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
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.
Bug
When multiple list items are selected and the user clicks the indent or
outdent toolbar button (or presses Tab / Shift+Tab), only the first
selected item changes its nesting level. All other selected items are ignored.
Root cause
Both
increaseNestingLevel()anddecreaseNestingLevel()insrc/trix/models/composition.jscallthis.getBlock(), which always returnsthe single block at
locationRange[0].index— the start of the selection.They then call
this.document.replaceBlock()exactly once and return.Fix
Iterate from
locationRange[0].indextolocationRange[1].index, applyingreplaceBlock()for each block and threading the updated document through theloop. A single
setDocument()call is made at the end.The same change is applied to
decreaseNestingLevel().Tests
Two regression tests are added to
src/test/system/list_formatting_test.js:3-item bullet list, selects all three, clicks increaseNestingLevel, and
asserts every block gained an extra nesting level.
3-item list where items 2 and 3 are nested, selects both, clicks
decreaseNestingLevel, and asserts both items returned to the top level.