feat(core): let Selector, MultiSelector, and ComplexSelector drop the trigger chevron - #5318
Open
ernestt wants to merge 1 commit into
Open
feat(core): let Selector, MultiSelector, and ComplexSelector drop the trigger chevron#5318ernestt wants to merge 1 commit into
ernestt wants to merge 1 commit into
Conversation
… trigger chevron The chevron renders as a sibling of the trigger button, after the optional clear button, so a selector with hasClear and a value showed both a × and a chevron in the same slot. StyleX has no descendant selectors and stylex.when.* only reads upward, so an xstyle on the field could not reach the icon — there was no sanctioned way to suppress it. hasChevron defaults to true, so existing selectors render identically. It gates only the chevron: a status glyph shares that slot and still appears. The chevron is decorative (aria-hidden) and outside the button, so the accessible name, focus order, and keyboard behaviour are unaffected. Named to match DropdownMenu's existing hasChevron rather than hasIndicator, which would collide with indicatorPosition — already the selected mark inside an option row on Selector and MultiSelector. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsComplexSelector (@astryxdesign/core) · View in Storybook
MultiSelector (@astryxdesign/core) · View in Storybook
Selector (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
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.
Problem
The chevron renders as a sibling of the trigger button, after the optional
InputClearButton. So withhasClearand a value selected, the trigger shows both a×and a chevron in the same end slot, and consumers who wanted the clear affordance to replace the chevron had no way to say so:astryx-selector-indicator-icon) only carriesdata-state="expanded|collapsed".data-variant,data-size,data-status,data-disabled— nothing for "has a value".stylex.when.*only reads upward, so anxstyleon the field cannot reach the icon.No sanctioned escape hatch existed.
API added
One optional boolean on all three components:
hasChevron = truein the destructure (the component idiom here —DropdownMenuuseshasChevron = true,SelectorusesindicatorPosition = 'end'; the?? trueform in feat(Table): let useTableSelection opt out of the checked-row highlight #5310 was for a hook config object, which has no destructure to default). A consumer that omits it renders identically.SelectorandMultiSelectorthe end slot is a ternary: a status glyph renders instead of the chevron when a status is attached.hasChevrongates the chevron arm alone — a status glyph reports on the value, so suppressing it would hide information rather than an affordance. With an attached status the flag is a no-op, which is asserted.Iconwithout alabelemitsaria-hidden="true", and the chevron is a sibling of the<button>, not a descendant — so it is neither in the a11y tree nor part of the button's name computation. Both facts are asserted in tests, alongside accessible name,textContent, and tab order being identical across the two modes.Why
hasChevron, nothasIndicatorindicatoris already taken on two of the three components, meaning something else.SelectorandMultiSelectorboth exposeindicatorPosition— which edge of an option row inside the popup carries the selected mark (a check on Selector, a checkbox on MultiSelector).Selector.tsximportsuseIndicatorandIndicatorPositionfrom theIndicatormodule for exactly that.hasIndicator={false}next toindicatorPosition="start"would read as "turn off the option checkmarks."hasChevronis the existing house name for this exact affordance:DropdownMenualready shipshasChevron?: booleandefaulting totrueto suppress its trigger chevron — and its theme target is likewise namedastryx-dropdown-menu-indicator-icon, so the theme-target/prop-name split is precedent, not a new inconsistency. It also matches thehas*convention for presentational sub-affordances (hasClear,hasSearch,hasHover,hasRowHighlight).Per-component notes
They are consistent in name, default, and semantics, but the trigger markup differs in two ways worth flagging:
ComplexSelectorhas nohasClear. Its end slot holds only the chevron, so the clear-replaces-chevron motivation does not apply. The flag is still useful (a trigger whose composed content supplies its own end affordance), and its doc example says so rather than reusing the clear-button framing.ComplexSelectorrenders no on-field status glyph, so its chevron was unconditional. It becomes{hasChevron && <Icon …/>}; the other two become a third ternary arm.Docs
Per-file, matching each doc's actual shape rather than assuming three surfaces everywhere:
Selector.doc.mjs—docs.props+ a newexamplesentry. ItsdocsZh/docsDenseare usage-only overlays with nopropDescriptionsat all; adding a lone Chinese entry would have been the only one in the file, so it was left alone.MultiSelector.doc.mjs— all three:docs…props,docsZh…propDescriptions(translated),docsDense…propDescriptions, plus anexamplesentry.ComplexSelector.doc.mjs—docs…props+docsDense.propDescriptions(it has nodocsZh), plus anexamplesentry.SYNC:headers were checked on all three sources — each already lists its own.doc.mjs, so unlike #5310 there was no missing entry to add. The lists also name storybook stories andpackages/cli/assets/templates/blocks/**; neither needs a change, since the prop is additive with an unchanged default (and templates were out of bounds for this PR).Changeset:
[feat]/patch, per the 0.x couplingcheck:changesetsenforces (only[breaking]may bump the minor while 0.x).Test plan
19 tests added — 7 on
Selector, 7 onMultiSelector, 5 onComplexSelector:hasChevron={false}removes it (all three)hasClear+ a value, the clear button is still present and the chevron is gone (Selector, MultiSelector)textContent, and tab order are identical across both modes (all three)aria-hidden="true"and is not contained by the trigger button (all three)aria-expandedstill flips with the chevron off (all three)Non-vacuity was checked, not assumed. I stashed only the three
.tsxsources, kept the tests, and re-ran: 11 of the 19 fail without the change. The 8 that pass either way do so by construction, and I want to be straight about which:Every test that asserts the opt-out itself fails without the change; each
hasChevron={false}render asserts the chevron is absent so none of them can drift vacuous.Gates:
pnpm -F @astryxdesign/core typecheckandtypecheck:docs— cleaneslinton all 10 changed files atASTRYX_STRICT_LINT=1— 0 errors (the one warning, unusedborderVarsinMultiSelector.tsx, reproduces onorigin/mainand is untouched here)prettier --checkon all 10 changed files — cleanvitest run— Selector 150 passed, MultiSelector 112 passed, ComplexSelector 17 passed (279 total, 3 files)pnpm check:repo— exit 0astryx component <Name> --denserenders the new prop row and example for all threeMade with Cursor