Skip to content

feat: admit Select - #37

Merged
inureyes merged 1 commit into
mainfrom
feat/admit-select
Sep 8, 2026
Merged

inureyes merged 1 commit into
mainfrom
feat/admit-select

Conversation

@inureyes

@inureyes inureyes commented Sep 8, 2026

Copy link
Copy Markdown
Member

Admits Select, proposed in #16.

The proposal asked two questions. Both are answered here rather than left open, and both are reversible if a maintainer disagrees: this is additive, and no existing component changes.

Is it wanted? Yes, and rule 2 is the reason. backend.ai-go and continuum-hub both ship this component, with the same SelectOption shape and the same core props. That is what the second-consumer test asks for, and it is the part that usually fails: the same check disqualified MultiSelect, which exists in one product, and TextInput, where the two products share a directory name and nothing else.

Superset or intersection? The superset: the label, the search filter, invalid, and aria-describedby come too. An intersection-only admission would leave the hub wrapping this package to re-add them, which is the fork the admission exists to end. The extra props are generic view models rather than one product's categories, and backend.ai-go reads them as additive, so it can adopt without touching a call site.

What changed on the way in

The locale key. The empty-search line called t("common.select.noOptions"). It is noOptionsLabel now, with an English default, the shape closeLabel and moreTabsLabel already use. That was the one thing the proposal named as blocking rule 1.

The focus border. .select__trigger:focus and .select__search-input:focus-visible painted their border from the bare --token-colorPrimary. That accent measures 2.61:1 against white in orange-light, under the 3:1 that WCAG 2.2 SC 1.4.11 sets for a focus indicator. They mix the accent with the text colour now, which is the form CONTRIBUTING documents for a border-drawn ring. This package's own guard caught it on the first test run, which is a fair advertisement for the guard: the component had shipped in two products with that border.

Nothing else in the CSS differs from the hub's copy. The diff is those two declarations.

Three tokens. --token-colorTextDisabled, --token-colorTextPlaceholder and --token-colorPrimaryBgHover were read but undeclared, which the token contract guard refuses. The first two are #767676 rather than the customary #bfbfbf, which measures 2.2:1 on white: a placeholder naming a field and a disabled value explaining why it cannot be changed are both text a reader is expected to read.

Tests

Fifteen, where both products had none. That is rule 3, and it is not paperwork: the point of sharing a component is being able to change it afterwards.

They cover the selected value and the open state, aria-selected on the right option, selection by click and by Enter, arrow-key movement through aria-activedescendant, Escape closing without choosing, a disabled option staying visible and unselectable, the search filter over both label and description, the empty-search line in English and in a caller's own words, the label naming the trigger, invalid, aria-describedby, and onBlur.

Three perturbations, each verified: hardcoding the empty label fails one, dropping aria-invalid fails one, ignoring option.disabled fails one.

One shim in the test setup: jsdom implements no layout and therefore no scrollIntoView, which the component calls to keep the active option visible.

pnpm run verify passes: 535 tests, 140 files packed.

Not in this pull request

The hub still has its copy; retiring it is lablup/continuum-hub#1001's half, and it needs this released first. --token-controlLabelOffset, held back in #15 for want of a labelled control, is now worth revisiting, and still depends on --token-lineHeightSM, which this package does not declare.

Refs #16

A single-select listbox with an optional search filter, icons, descriptions, a disabled-but-visible option state, a field label, and the `invalid` and `aria-describedby` props a form needs. Proposed in #16 before any code, as CONTRIBUTING asks.

Rule 2 is what made it admissible, and it is the part that usually fails: `backend.ai-go` and `continuum-hub` both ship this component, with the same `SelectOption` shape and the same core props, the hub's being the superset. The same check disqualified `MultiSelect`, which exists in one product, and `TextInput`, where the two products share a directory name and nothing else.

The superset rather than the intersection. An intersection-only admission would leave the hub wrapping the package to re-add the label, the filter and the validity flag, which is the fork this replaces; and the extra props are generic view models, not one product's categories. `backend.ai-go` reads them as additive, so it can adopt without touching a call site.

Two changes on the way in. The empty-search line resolved a product locale key, and is now `noOptionsLabel` with an English default, the shape `closeLabel` and `moreTabsLabel` already use. The focus border was painted from the bare accent, which measures 2.61:1 against white in one shipped family; it mixes the accent with the text colour now, which is what this package requires of a border-drawn ring, and the guard that says so caught it on the first run.

Fifteen tests, which is fifteen more than either product had. Rule 3 is the reason: the whole point of sharing a component is being able to change it later.

`--token-colorTextDisabled`, `--token-colorTextPlaceholder` and `--token-colorPrimaryBgHover` join `styles/base.css`, which the token contract guard required. The first two are #767676 rather than the customary #bfbfbf, which measures 2.2:1: a placeholder naming a field and a disabled value explaining itself are both read.

Refs #16
@inureyes
inureyes merged commit d356eed into main Sep 8, 2026
3 checks passed
@inureyes
inureyes deleted the feat/admit-select branch September 8, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant