Replace hardcoded toolbar breakpoints with KListWithOverflow#5707
Replace hardcoded toolbar breakpoints with KListWithOverflow#5707nucleogenesis wants to merge 1 commit intolearningequality:unstablefrom
Conversation
Use KDS KListWithOverflow component to dynamically collapse toolbar items into a "More" dropdown based on available space instead of fixed pixel breakpoints. - Integrate KListWithOverflow for automatic overflow detection - Add overflow hidden CSS to prevent flicker during resize - Simplify toolbar logic by removing manual breakpoint calculations Fixes learningequality#5258 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Clean replacement of hardcoded toolbar breakpoints with KListWithOverflow. The approach is solid — delegating overflow measurement to KDS removes brittle pixel breakpoints and makes the toolbar resilient to future button additions.
CI passing. Screenshots verified across viewports — toolbar collapses gracefully and the overflow dropdown renders correctly.
- suggestion:
::v-deep .listtargets KDS internals (line 524) — could break on KDS updates. Worth a comment noting the dependency. - suggestion: Three aria-label strings are hardcoded in English rather than using the i18n system. Pre-existing from the previous implementation, but worth addressing as a follow-up.
- praise: The
isFromOverflowcheck viamoreDropdown.value?.contains(event.target)(line 403) is a nice improvement — it directly detects overflow context instead of using the old proxy check onvisibleCategories.
| } | ||
|
|
||
| /* Clip items that wrap during resize recalculation to prevent flicker */ | ||
| .overflow-list ::v-deep .list { |
There was a problem hiding this comment.
suggestion: This ::v-deep .list selector reaches into KDS internals to prevent flicker. It works, but it's fragile — if KDS changes the class name or structure in a future version, this breaks silently. Consider adding a brief comment noting which KDS version/component this targets, so future updates can verify it still applies.
| ref="moreDropdownContainer" | ||
| class="more-dropdown-container" | ||
| role="group" | ||
| :aria-label="'More options'" |
There was a problem hiding this comment.
suggestion: 'More options' and 'Additional formatting options' (line 179) are hardcoded English strings. These were carried over from the previous implementation, but since this PR is already touching this area, it would be a good opportunity to add them to TipTapEditorStrings.js for proper i18n. Not blocking since it's pre-existing.
| // If the tool is in the overflow menu, we center the modal | ||
| if (!visibleCategories.value.includes('insert')) target = null; | ||
| // If the tool is in the overflow menu (clicked from dropdown), center the modal | ||
| const isFromOverflow = moreDropdown.value?.contains(event.target); |
There was a problem hiding this comment.
praise: Good improvement — directly checking moreDropdown.value?.contains(event.target) is more robust than the old proxy check on visibleCategories. This correctly handles the case regardless of which specific groups are overflowed.
2 participants
Summary
Replace hardcoded pixel breakpoints in the RTE toolbar with dynamic
KListWithOverflowfrom KDS. This allows the toolbar to automatically collapse items into a "More" dropdown based on available space, rather than using fixed breakpoints.Changes:
KListWithOverflowcomponent to dynamically measure and collapse toolbar itemsoverflow: hiddenon.list) to prevent flicker during resize recalculationCloses: #5258
Screenshots
Test Plan