Standardize dropdown trigger arrow, sizing, and primary focus ring#5105
Closed
lukemelia wants to merge 1 commit into
Closed
Standardize dropdown trigger arrow, sizing, and primary focus ring#5105lukemelia wants to merge 1 commit into
lukemelia wants to merge 1 commit into
Conversation
Addresses three boxel-ui trigger-polish items from design review: CS-11326: the sort menu used a different arrow (filled DropdownArrowDown) than boxel-ui's form controls and never flipped on open. Switch the sort and selection-menu triggers to the shared CaretDown chevron (already used by select/multi-select/picker/dropdown) and flip it on open via the trigger's aria-expanded state. CS-11327: give the selection-menu count a reserved, tabular-figure width so the trigger doesn't shift when the count crosses 1↔2 digits (e.g. a Select All jumping 9→10). CS-11328: make the primary BoxelButton focus ring robust and correct — set the full outline shorthand (not just the color, which relied on the UA default outline-style) 2px outside the button, and darken the ring to match when the fill darkens on hover/active. The selection-menu's local focus-ring override is now redundant and removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Standardizes dropdown trigger polish in boxel-ui for the card-chooser/toolbar surfaces by unifying caret iconography, ensuring the sort trigger caret flips when open, stabilizing the selection count width to prevent layout shift, and making the design-system focus ring explicit and consistent across browsers.
Changes:
- Swaps sort/selection triggers from
DropdownArrowDownto the thinnerCaretDownand flips the sort caret viaaria-expanded. - Stabilizes the
SelectionMenucount width (tabular digits +2chminimum) to avoid trigger shifting as the count changes. - Updates
BoxelButton:focus-visibleto set the fulloutlineshorthand (and darken the primary ring on hover/active) instead of relying on UA default outline-style.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/boxel-ui/addon/src/components/sort-dropdown/index.gts | Uses CaretDown and adds open-state caret flip styling keyed off aria-expanded. |
| packages/boxel-ui/addon/src/components/selection-menu/index.gts | Uses CaretDown, removes redundant local focus ring, and stabilizes count width to prevent layout shift. |
| packages/boxel-ui/addon/src/components/button/index.gts | Makes focus ring explicit via outline shorthand and aligns primary ring color with hover/active fill. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
[Claude Code 🤖] Split into three stacked, single-issue PRs (byte-identical to this combined diff):
Closing this in favor of those. |
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.
Three boxel-ui trigger-polish items from Burcu's review of #5083, scoped to the card-chooser/toolbar triggers (not the unrelated host chrome).
CS-11326 — one standard arrow + flip on open
DropdownArrowDownand never flipped on open (the regression). boxel-ui's form controls (select, multi-select, picker, dropdown) already use the thinCaretDownchevron and flip.sort-dropdownandselection-menutriggers toCaretDown, and the sort button now flips its caret on open via itsaria-expandedstate.DropdownArrowDownusages (file tree, submode switcher, realm dropdown, etc.) are intentionally left alone.CS-11327 — trigger doesn't shift when count changes
selection-menucount now reserves a stable 2-ch, tabular-figure width, so the trigger no longer jumps when the count crosses 1↔2 digits (e.g. a Select All going 9→10).--boxel-button-smmin-height; the old "squished padding" was the pre-rebuild trigger.CS-11328 — primary button focus ring
BoxelButton:focus-visiblenow sets the full outline shorthand (--boxel-outline-width/-style, i.e. 2px solid) instead of onlyoutline-color— previously it relied on the UA default outline-style. Ring sits 2px outside.selection-menu's local focus-ring override is now redundant and removed.:focus-visiblechange is on the sharedBoxelButton, so every button's keyboard focus ring is now explicitly2px solidrather than the browser default — intended (it's the design-system--boxel-outline).Base branch
Targets
cs-11325-…(PR #5100, the sharedSelectionMenu), same as #5102 — keeps this diff scoped. Will rebase ontomainand retarget once #5100 lands.Testing
selection-menutest targets the checkmark'scircle(the caret has none, so it stays unambiguous).🤖 Generated with Claude Code