Extract SelectionMenu into boxel-ui as a shared component#5101
Closed
lukemelia wants to merge 2 commits into
Closed
Extract SelectionMenu into boxel-ui as a shared component#5101lukemelia wants to merge 2 commits into
lukemelia wants to merge 2 commits into
Conversation
The "N selected" bulk-selection trigger was a flat teal pill that read as a regression from the previous dropdown. Keep BoxelDropdown + Menu (the right primitives for an action menu) and restyle the trigger as a proper primary dropdown button, modeled on the boxel-ui primary Button / highlight ContextButton: - highlight fill with --boxel-highlight-foreground text, deepening to --boxel-highlight-hover on hover and while the menu is open - roomier internal spacing and a taller min-height so the content is no longer squished - keyboard focus draws a ring outside the button and no longer darkens the fill (deepening is reserved for hover / open) - the caret flips to point up while the menu is open Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
6e3f470 to
639345b
Compare
The bulk-selection control (primary dropdown trigger + action menu) lived inline in the card chooser's results header. Extract it into a generic boxel-ui SelectionMenu so any surface — including the base CardsGrid card, which can only import boxel-ui — can adopt it. The trigger is a standard primary BoxelButton (inheriting the design system's highlight colors, hover, and focus-ring), with a leading selection checkmark, the count, and a caret that flips while the menu is open. The component is content-agnostic: the consumer supplies the menu items via @Items, so Select All / Deselect All and the inert count header stay in the app (built in SearchResultHeader) rather than the design system. Also move the shared SelectionCheckmark artwork into boxel-ui and reuse it in CardHeader, removing a duplicated inline SVG (advances CS-11333). No behavior change for the card chooser: same markup, data-test hooks, aria-label, and menu items. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
639345b to
3e1120c
Compare
ed0f9c7 to
2746dc8
Compare
Contributor
Author
|
[Claude Code 🤖] Folding this into #5100 for review — the extraction reworks the same selection trigger that PR introduces, so reviewing them together avoids reviewing intermediate state that this PR immediately replaces. #5100 now contains the full change (squashed): the primary-dropdown trigger + the boxel-ui SelectionMenu extraction + the SelectionCheckmark dedup. |
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.
Prep for bringing the bulk-selection menu to the workspace cards grid. Extract the selection control out of the card chooser's results header into a generic boxel-ui
SelectionMenu, so any surface can adopt it — including the baseCardsGridcard, which can only import boxel-ui (not host).The trigger is a primary
BoxelButtonRather than a bespoke
<button>, the trigger is a standardButton @kind='primary', so it inherits the design system's highlight colors, hover, and focus-ring behavior. The component's scoped CSS only adds what Button doesn't: the gap between the checkmark / count / caret, the readable highlight foreground (Button's primary text defaults to--boxel-dark), a tighter radius, the open-state deepening, and the caret flip.Generic / content-agnostic
The caller supplies the menu contents via
@items, so Select All / Deselect All (and the inert count header) stay in the app — built inSearchResultHeaderand handed in — and app-specific selection semantics don't leak into the design system.@labelsets the trigger's accessible name (the card chooser passes "N cards selected").Shared icon + card-header dedup
Moved the
SelectionCheckmarkartwork into boxel-ui (a two-color composite, so it's a component, not part of the generated monochrome icon set) and pointedCardHeaderat it — removing a byte-identical inline SVG and advancing CS-11333.Verification
boxel-ui + host both pass
ember-tsc(0 errors), eslint, and prettier; existing selection-menu tests' markup/data-test/aria-label are unchanged. The trigger's rendering now derives fromButton(radius/height/hover), so it's worth a fresh in-browser glance.Stacked on #5100 → #5099; auto-retargets to
mainas those merge. (The workspace-grid integration — wiring host selection state into the base card's toolbar — is the follow-up Adorn UI issue.)🤖 Generated with Claude Code