Skip to content

Commit 1da87f8

Browse files
fix(select): align contract and runtime behavior (#304)
* fix(select): align contract and runtime behavior * fix(select): update API report
1 parent 69b32ab commit 1da87f8

File tree

10 files changed

+104
-31
lines changed

10 files changed

+104
-31
lines changed

docs/widgets/catalog.json

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,18 @@
301301
},
302302
{
303303
"name": "select",
304-
"requiredProps": ["id", "value", "options", "onChange"],
305-
"commonProps": ["placeholder", "disabled"],
304+
"requiredProps": ["id", "value", "options"],
305+
"commonProps": [
306+
"onChange",
307+
"placeholder",
308+
"disabled",
309+
"focusable",
310+
"accessibleLabel",
311+
"focusConfig",
312+
"dsVariant",
313+
"dsTone",
314+
"dsSize"
315+
],
306316
"example": "ui.select({ id: 'country', value, options, onChange })"
307317
},
308318
{

docs/widgets/select.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Select
22

3-
A focusable dropdown selection widget for choosing one value from a list of options.
3+
A focusable inline selection widget that cycles through one value from a list of options.
44

55
## Usage
66

@@ -24,11 +24,13 @@ ui.select({
2424
| Prop | Type | Default | Description |
2525
|---|---|---|---|
2626
| `id` | `string` | **required** | Unique identifier for focus and event routing |
27+
| `focusable` | `boolean` | `true` | Opt out of Tab focus while keeping id-based routing available |
28+
| `accessibleLabel` | `string` | - | Semantic label used for accessibility and focus announcements |
2729
| `value` | `string` | **required** | Currently selected value |
2830
| `options` | `{ value: string; label: string; disabled?: boolean }[]` | **required** | Available options |
2931
| `onChange` | `(value: string) => void` | - | Called when selection changes |
3032
| `disabled` | `boolean` | `false` | Disable focus and interaction |
31-
| `placeholder` | `string` | - | Text shown when no matching option label is found |
33+
| `placeholder` | `string` | `"Select..."` | Text shown when the current value has no matching option label |
3234
| `focusConfig` | `FocusConfig` | - | Control focus visuals; `{ indicator: "none" }` suppresses focused select decoration |
3335
| `dsVariant` | `"solid" \| "soft" \| "outline" \| "ghost"` | - | Reserved for future select recipes (currently ignored) |
3436
| `dsTone` | `"default" \| "primary" \| "danger" \| "success" \| "warning"` | - | Reserved for future select recipes (currently ignored) |
@@ -51,9 +53,11 @@ If the active theme does not provide semantic color tokens, selects fall back to
5153
- Focusable when enabled.
5254
- **Mouse click** focuses the select widget.
5355
- Navigate options with **ArrowUp/ArrowDown**.
54-
- Confirm selection with **Enter**.
56+
- **Enter** and **Space** advance to the next enabled option.
5557
- **Tab / Shift+Tab** moves focus to the next/previous widget.
5658

59+
This widget does not open a popup or dropdown list. It renders the current label inline and cycles through enabled options.
60+
5761
## Examples
5862

5963
### 1) With a `field` wrapper

packages/core/etc/core.api.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5230,7 +5230,6 @@ export type SelectProps = Readonly<{
52305230
onChange?: (value: string) => void;
52315231
disabled?: boolean;
52325232
placeholder?: string;
5233-
error?: boolean;
52345233
focusConfig?: FocusConfig;
52355234
dsVariant?: WidgetVariant;
52365235
dsTone?: WidgetTone;
@@ -8014,7 +8013,7 @@ export type ZrUiErrorCode = "ZRUI_INVALID_STATE" | "ZRUI_MODE_CONFLICT" | "ZRUI_
80148013
// src/runtime/instances.ts:161:3 - (ae-forgotten-export) The symbol "AppStateSelection" needs to be exported by the entry point index.d.ts
80158014
// src/runtime/localState.ts:401:3 - (ae-forgotten-export) The symbol "TreeFlatCache" needs to be exported by the entry point index.d.ts
80168015
// src/runtime/localState.ts:406:3 - (ae-forgotten-export) The symbol "TreePrefixCache" needs to be exported by the entry point index.d.ts
8017-
// src/runtime/widgetMeta/focusInfo.ts:7:3 - (ae-forgotten-export) The symbol "RuntimeInstance" needs to be exported by the entry point index.d.ts
8016+
// src/runtime/widgetMeta/focusInfo.ts:8:3 - (ae-forgotten-export) The symbol "RuntimeInstance" needs to be exported by the entry point index.d.ts
80188017
// src/testing/renderer.ts:93:3 - (ae-forgotten-export) The symbol "TestRecordedOp" needs to be exported by the entry point index.d.ts
80198018
// src/widgets/composition.ts:88:3 - (ae-forgotten-export) The symbol "UnknownCallback" needs to be exported by the entry point index.d.ts
80208019
// src/widgets/style.ts:26:3 - (ae-forgotten-export) The symbol "UnderlineStyle" needs to be exported by the entry point index.d.ts

packages/core/src/app/__tests__/widgetRenderer.integration.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,59 @@ describe("WidgetRenderer integration battery", () => {
621621
assert.deepEqual(values, []);
622622
});
623623

624+
test("select uses Enter and Space to advance like the inline cycler runtime", () => {
625+
const backend = createNoopBackend();
626+
const renderer = new WidgetRenderer<void>({
627+
backend,
628+
requestRender: () => {},
629+
});
630+
631+
let value = "dark";
632+
const values: string[] = [];
633+
const view = () =>
634+
ui.select({
635+
id: "theme",
636+
value,
637+
options: [
638+
{ value: "dark", label: "Dark" },
639+
{ value: "light", label: "Light" },
640+
{ value: "system", label: "System" },
641+
],
642+
onChange: (nextValue) => {
643+
value = nextValue;
644+
values.push(nextValue);
645+
},
646+
});
647+
648+
let res = renderer.submitFrame(
649+
() => view(),
650+
undefined,
651+
{ cols: 40, rows: 5 },
652+
defaultTheme,
653+
noRenderHooks(),
654+
);
655+
assert.ok(res.ok);
656+
657+
renderer.routeEngineEvent(keyEvent(3 /* TAB */));
658+
assert.equal(renderer.getFocusedId(), "theme");
659+
660+
renderer.routeEngineEvent(keyEvent(2 /* ENTER */));
661+
assert.equal(value, "light");
662+
663+
res = renderer.submitFrame(
664+
() => view(),
665+
undefined,
666+
{ cols: 40, rows: 5 },
667+
defaultTheme,
668+
noRenderHooks(),
669+
);
670+
assert.ok(res.ok);
671+
672+
renderer.routeEngineEvent(keyEvent(32 /* SPACE */));
673+
assert.equal(value, "system");
674+
assert.deepEqual(values, ["light", "system"]);
675+
});
676+
624677
test("focusTrap wraps TAB within active trap", () => {
625678
const backend = createNoopBackend();
626679
const renderer = new WidgetRenderer<void>({

packages/core/src/renderer/renderToDrawlist/widgets/renderFormWidgets.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
selectRecipe,
1414
sliderRecipe,
1515
} from "../../../ui/recipes.js";
16+
import { DEFAULT_PLACEHOLDER, getSelectDisplayText } from "../../../widgets/select.js";
1617
import {
1718
DEFAULT_SLIDER_TRACK_WIDTH,
1819
formatSliderValue,
@@ -873,19 +874,13 @@ export function renderFormWidgets(
873874
!disabled,
874875
);
875876
const value = typeof props.value === "string" ? props.value : "";
876-
const placeholder = typeof props.placeholder === "string" ? props.placeholder : "Select…";
877+
const placeholder =
878+
typeof props.placeholder === "string" ? props.placeholder : DEFAULT_PLACEHOLDER;
877879

878880
const options = Array.isArray(props.options)
879881
? (props.options as readonly SelectOption[])
880882
: [];
881-
let label = "";
882-
for (const opt of options) {
883-
if (opt && opt.value === value) {
884-
label = opt.label;
885-
break;
886-
}
887-
}
888-
if (label.length === 0) label = placeholder;
883+
const label = getSelectDisplayText(value, options, placeholder);
889884

890885
const colorTokens = getColorTokens(theme);
891886
const dsSize = readWidgetSize(props.dsSize) ?? "md";

packages/core/src/runtime/__tests__/widgetMeta.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,22 @@ test("widgetMeta: collectAllWidgetMetadata includes accessible focus semantics",
409409
assert.equal(ageInfo.announcement, "Age — Required");
410410
});
411411

412+
test("widgetMeta: select visible label falls back to placeholder for unknown value", () => {
413+
const vnode = ui.select({
414+
id: "theme",
415+
value: "missing",
416+
placeholder: "Choose a theme",
417+
options: [{ value: "dark", label: "Dark" }],
418+
});
419+
420+
const committed = commitTree(vnode);
421+
const allMeta = collectAllWidgetMetadata(committed);
422+
const info = allMeta.focusInfoById.get("theme");
423+
assert.ok(info);
424+
assert.equal(info.visibleLabel, "Choose a theme");
425+
assert.equal(info.announcement, "Choose a theme");
426+
});
427+
412428
test("widgetMeta: reusable collector publishes stable snapshots across collects", () => {
413429
const collector = createWidgetMetadataCollector();
414430

packages/core/src/runtime/widgetMeta/focusInfo.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getSelectDisplayText } from "../../widgets/select.js";
12
import type { RuntimeInstance } from "../commit.js";
23
import { readNonEmptyString } from "./helpers.js";
34

@@ -42,16 +43,13 @@ function readFocusableVisibleLabel(vnode: RuntimeInstance["vnode"]): string | nu
4243
const options = Array.isArray(props.options)
4344
? (props.options as readonly { value?: unknown; label?: unknown }[])
4445
: [];
45-
for (const option of options) {
46-
if (
47-
typeof option?.value === "string" &&
48-
option.value === value &&
49-
typeof option.label === "string"
50-
) {
51-
return readNonEmptyString(option.label);
52-
}
53-
}
54-
return readNonEmptyString(props.placeholder);
46+
return readNonEmptyString(
47+
getSelectDisplayText(
48+
value,
49+
options as readonly { value: string; label: string; disabled?: boolean }[],
50+
readNonEmptyString(props.placeholder) ?? undefined,
51+
),
52+
);
5553
}
5654
case "radioGroup": {
5755
const props = vnode.props as { value?: unknown; options?: unknown };

packages/core/src/widgets/__tests__/formWidgets.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ describe("select widget utilities", () => {
9494
assert.equal(text, "United Kingdom");
9595
});
9696

97-
test("getSelectDisplayText returns value for unknown value", () => {
97+
test("getSelectDisplayText returns placeholder for unknown value", () => {
9898
const text = getSelectDisplayText("unknown", options);
99-
assert.equal(text, "unknown");
99+
assert.equal(text, DEFAULT_PLACEHOLDER);
100100
});
101101

102102
test("findOptionIndex finds correct index", () => {

packages/core/src/widgets/select.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export function getSelectDisplayText(
4141
}
4242

4343
const option = options.find((opt) => opt.value === value);
44-
return option?.label ?? value;
44+
return option?.label ?? placeholder ?? DEFAULT_PLACEHOLDER;
4545
}
4646

4747
/**

packages/core/src/widgets/types/forms.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ export type SelectProps = Readonly<{
4848
disabled?: boolean;
4949
/** Placeholder text when no value is selected. */
5050
placeholder?: string;
51-
/** Whether to show the select in an error state. */
52-
error?: boolean;
5351
/** Optional focus appearance configuration. */
5452
focusConfig?: FocusConfig;
5553
/** Design system: visual variant (reserved for future select recipes). */

0 commit comments

Comments
 (0)