feat(selector): add dropdown-internals theme targets for Selector & MultiSelector - #4596
feat(selector): add dropdown-internals theme targets for Selector & MultiSelector#4596freddymeta wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsMultiSelector (@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 |
0911b0d to
cccb37c
Compare
d23f875 to
60cbc17
Compare
cixzhang
left a comment
There was a problem hiding this comment.
Two asks before this lands:
1. Split out the theming. The dropdown-internals theme targets (selector-dropdown / -search / -search-input / -empty / -section-header and the multi-selector equivalents) are purely additive and low-risk — let's land those as their own PR. It's much easier to review/merge and stays out of the way of the other Selector-family theme-target PRs in flight (#4626, #4628).
2. Search icon + clear button should be built in, not opt-in props. Rather than adding hasSearchClear, let's make the dropdown search field just have the clear (×) button and a search magnifier as part of the component — a searchable dropdown wanting a clear affordance and a search glyph is the expected default, not something each consumer opts into. That keeps the API surface smaller and behavior consistent everywhere instead of gating standard search-field affordances behind a flag (same reasoning we've been applying elsewhere — prefer built-in correct behavior over an options knob).
So: theme targets → their own PR; the search-field clear button + icon → built into the component directly (no hasSearchClear prop).
60cbc17 to
911583a
Compare
…ultiSelector Expose stable theme targets for the searchable dropdown internals so consumers can restyle them via defineTheme instead of fragile structural CSS. New targets on both components: the dropdown popover content wrapper, the search wrapper and its input, the empty state, and grouped-option section headers. Purely additive themeProps on existing elements; default appearance is unchanged.
911583a to
dd718a0
Compare
|
Closing in favor of #4636, which reworks the dropdown search field into a The non-search targets proposed here ( |
What
Expose stable theme targets for the searchable-dropdown internals of Selector and MultiSelector, so consumers can restyle them via
defineThemeinstead of fragile structural CSS.New targets (on existing elements — purely additive
themeProps, default appearance unchanged):selector-dropdownmulti-selector-dropdownselector-searchmulti-selector-searchselector-search-inputmulti-selector-search-input<input>selector-emptymulti-selector-emptyselector-section-headermulti-selector-section-headerEach target is registered in the component's
.doc.mjstheming.targetsand covered by tests (thethemingTargetssuite validates every rendered target is documented).Why
Restyling dropdown internals previously required brittle descendant CSS that broke whenever the DOM structure shifted. These stable, named targets make dropdown theming first-class.
Notes
This PR is the theming half of a split, per review feedback that the low-risk, purely-additive theme targets should land on their own (easier to review, avoids collision with in-flight theming PRs). The search-affordances half (a built-in search magnifier + clear button in the dropdown search field) is a separate PR that composes with these targets — it adds its own
*-search-icon/*-search-clear-icontargets and does not touch the targets here.Testing
Selector+MultiSelectorsuites and thethemingTargetssuite pass.typecheck,typecheck:docs, storybook typecheck, docsitegenerate+test,sync-exports --check,check-sync,check-changesets,generate-token-docs --check, and eslint on the changed files all pass locally.