From 6ff473171dc87686088c1e10e53ad27ece0f8e84 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 18:58:12 +0000 Subject: [PATCH 1/2] fix(components): combobox honours options[].disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `options[].disabled` was declared by `@object-ui/types` and validated by the zod mirror, but `Combobox` never read it: each option became a `CommandItem` carrying `key`, `value` and `onSelect` only, so an option authored `disabled: true` passed validation, type-checked against the published `ComboboxSchema`, and rendered as an ordinary selectable option. The component now sets `disabled={option.disabled}`, the spelling the sibling select renderer already uses on `SelectItem`. cmdk 1.1.1 needs nothing more: a disabled `Command.Item` renders with `onClick` undefined, never registers the `cmdk-item-select` listener Enter dispatches, and is excluded from the valid-item selector the arrow keys walk — so a refusal inside `onSelect` would have been unreachable code. Measured, not assumed. The pin asserts both halves, because an attribute-only pin passes on "styled disabled but still clickable": the option carries `data-disabled` / `aria-disabled`, AND neither a click nor Enter fires `onValueChange`. Two control legs keep those negatives from being vacuous. Also refreshes the `ComboboxOption` block in the combobox docs page, which still documented the shape as `{ value; label }`. It sits in a plaintext fence, so no docs gate reads it. Refs: objectui#7687 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .changeset/7687-combobox-option-disabled.md | 32 ++++++ content/docs/components/form/combobox.mdx | 1 + .../combobox-option-disabled.test.tsx | 108 ++++++++++++++++++ packages/components/src/custom/combobox.tsx | 13 +++ 4 files changed, 154 insertions(+) create mode 100644 .changeset/7687-combobox-option-disabled.md create mode 100644 packages/components/src/__tests__/combobox-option-disabled.test.tsx diff --git a/.changeset/7687-combobox-option-disabled.md b/.changeset/7687-combobox-option-disabled.md new file mode 100644 index 0000000000..a3edd5dea9 --- /dev/null +++ b/.changeset/7687-combobox-option-disabled.md @@ -0,0 +1,32 @@ +--- +'@object-ui/components': minor +--- + +Honour `options[].disabled` on a `combobox` node (objectui#7687). + +**User-visible behaviour change, deliberately — hence `minor`, not `patch`.** The +member was already declared by `@object-ui/types` (`ComboboxOption.disabled`) and +already validated by the zod mirror (`ComboboxOptionSchema`, pinned as `boolean` +on both faces by the objectui#7087 twin-symmetry ruling), but the component never +read it: `Combobox` mapped each option to a `CommandItem` carrying `key`, `value` +and `onSelect` only. So an option authored `{ value, label, disabled: true }` +passed `safeValidateSchema`, type-checked against the published `ComboboxSchema`, +and then rendered as an ordinary, fully selectable option — a declared key with no +read site behind it, the class the enforce-or-remove ledgers exist to close. + +An author who already writes `disabled: true` today gets a different combobox +after this change: that option now renders dimmed and can no longer be chosen, by +click or by keyboard. That is the intended repair — declared and validated should +mean enforced — but it is a change in what existing metadata does, not a silent +internal fix, so it is priced as a behaviour change rather than a patch. + +The alternative remedy, retiring `disabled` from `ComboboxOption` and the zod +mirror, was weighed and **not** adopted: it narrows a published surface and would +require the objectui#7087 twin-symmetry pin to be changed, where honouring the key +restores declared = enforced at the cost of one prop. The spelling follows the +sibling select renderer, which already sets `disabled={opt.disabled}` on its +`SelectItem`. + +Nothing else moves. The whole-control `disabled` prop (the one forwarded to the +trigger button) is untouched, `@object-ui/types` is untouched, and no new key is +introduced — this release only starts reading one that was already published. diff --git a/content/docs/components/form/combobox.mdx b/content/docs/components/form/combobox.mdx index da2154c0af..ed6518e0fc 100644 --- a/content/docs/components/form/combobox.mdx +++ b/content/docs/components/form/combobox.mdx @@ -28,6 +28,7 @@ The Combobox component combines a text input with a dropdown list, allowing user interface ComboboxOption { value: string; label: string; + disabled?: boolean; // Option renders dimmed and cannot be selected } interface ComboboxSchema { diff --git a/packages/components/src/__tests__/combobox-option-disabled.test.tsx b/packages/components/src/__tests__/combobox-option-disabled.test.tsx new file mode 100644 index 0000000000..de369ec483 --- /dev/null +++ b/packages/components/src/__tests__/combobox-option-disabled.test.tsx @@ -0,0 +1,108 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#7687 — `options[].disabled` on a `combobox` node must be READ. + * + * The member was declared (`@object-ui/types` `ComboboxOption.disabled`), + * validated (`ComboboxOptionSchema` in the zod mirror, pinned as `boolean` on + * both faces by `disabled-twin-symmetry-7087.test.ts`) and never read: the + * component mapped each option to a `CommandItem` with `key` / `value` / + * `onSelect` only, so an option authored `disabled: true` passed validation, + * type-checked against the published `ComboboxSchema`, and rendered as an + * ordinary selectable option — a declared key with no read site behind it. + * + * ⛔ An attribute-only pin is not enough here: it passes on "styled disabled + * but still clickable", which is the exact defect this card is about. So the + * behaviour is pinned too — a disabled option must not fire `onValueChange`. + * The third test is the CONTROL that keeps that negative from being vacuous: + * without it, a popover that never opened would satisfy "not called". + */ +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { Combobox } from '../custom/combobox'; + +const OPTIONS = [ + { value: 'alpha', label: 'Alpha' }, + { value: 'beta', label: 'Beta', disabled: true }, + { value: 'gamma', label: 'Gamma', disabled: false }, +]; + +function openDropdown() { + fireEvent.click(screen.getByRole('combobox')); +} + +describe('Combobox honours options[].disabled (objectui#7687)', () => { + it('marks an option authored `disabled: true` as disabled in the DOM', () => { + render(); + openDropdown(); + + const beta = screen.getByRole('option', { name: /Beta/ }); + // `data-disabled` is what the CommandItem wrapper's className already + // styles (`data-[disabled=true]:opacity-50 …:pointer-events-none`); + // `aria-disabled` is what assistive tech and cmdk's own valid-item + // selector read. + expect(beta).toHaveAttribute('data-disabled', 'true'); + expect(beta).toHaveAttribute('aria-disabled', 'true'); + }); + + it('refuses to select a disabled option — no onValueChange', () => { + const onValueChange = vi.fn(); + render(); + openDropdown(); + + fireEvent.click(screen.getByRole('option', { name: /Beta/ })); + expect(onValueChange).not.toHaveBeenCalled(); + }); + + it('refuses the keyboard path too — Enter on a search narrowed to the disabled option selects nothing', () => { + const onValueChange = vi.fn(); + render(); + openDropdown(); + + // A separate code path from the click above: cmdk keeps disabled items out + // of the valid-item selector its arrow keys and its auto-select walk, and + // never registers the `cmdk-item-select` listener Enter dispatches. With + // the list narrowed to `Beta` alone there is nothing left to select. + const search = screen.getByPlaceholderText('Search...'); + fireEvent.change(search, { target: { value: 'bet' } }); + expect(screen.getByRole('option', { name: /Beta/ })).toBeInTheDocument(); + expect(screen.queryByRole('option', { name: /Alpha/ })).toBeNull(); + + fireEvent.keyDown(search, { key: 'Enter' }); + expect(onValueChange).not.toHaveBeenCalled(); + }); + + it('CONTROL — options without `disabled: true` stay selectable', () => { + const onValueChange = vi.fn(); + render(); + openDropdown(); + + // Omitted (`alpha`) and explicitly `false` (`gamma`) are both enabled. + const alpha = screen.getByRole('option', { name: /Alpha/ }); + const gamma = screen.getByRole('option', { name: /Gamma/ }); + expect(alpha).toHaveAttribute('data-disabled', 'false'); + expect(gamma).toHaveAttribute('data-disabled', 'false'); + + fireEvent.click(alpha); + expect(onValueChange).toHaveBeenCalledWith('alpha'); + }); + + it('CONTROL — the keyboard path still selects an enabled option', () => { + const onValueChange = vi.fn(); + render(); + openDropdown(); + + const search = screen.getByPlaceholderText('Search...'); + fireEvent.change(search, { target: { value: 'gam' } }); + fireEvent.keyDown(search, { key: 'Enter' }); + expect(onValueChange).toHaveBeenCalledWith('gamma'); + }); +}); diff --git a/packages/components/src/custom/combobox.tsx b/packages/components/src/custom/combobox.tsx index 6eae8e5468..7a47a9c759 100644 --- a/packages/components/src/custom/combobox.tsx +++ b/packages/components/src/custom/combobox.tsx @@ -115,6 +115,19 @@ export function Combobox({ { onValueChange?.(currentValue === value ? "" : currentValue) setOpen(false) From b0a6b3c8bb023f84c307bac1e3f98d7228834074 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 19:56:23 +0000 Subject: [PATCH 2/2] test(types): claim the new combobox disabled row in the docs census MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `component-docs-disabled-inherited-7239.test.ts` is a census over `content/docs/components`: it classifies every documented `disabled?:` row as INHERITED or INDEPENDENT and pins the population by measurement. Documenting `ComboboxOption.disabled` adds a real 23rd row, so the census failed with the row unclassified and the population one short. The row is claimed as INDEPENDENT on the file's own stated criterion, not by bumping the count: the shipped `ComboboxOption` declares `disabled` itself and does NOT extend `BaseSchema`, so the narrow `boolean` spelling on the page is the correct one and the second INDEPENDENT assertion holds for it unchanged. Counts move consistently — rows 22 to 23, independent 8 to 9, inherited unchanged at 14. A new `## Amendments` section records why the ledger moved, so a later reader can tell a deliberate claim from a number someone bumped to get back to green. It also corrects a belief the #7687 card recorded: the page's `plaintext` fence does keep the doc-TYPE gates from reading it, but this census reads the `.mdx` files as text, so a docs edit under `content/docs/components` that touches a `disabled` row is answerable to `packages/types`. Refs: objectui#7687 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .changeset/7687-combobox-option-disabled.md | 14 +++++++-- ...onent-docs-disabled-inherited-7239.test.ts | 31 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/.changeset/7687-combobox-option-disabled.md b/.changeset/7687-combobox-option-disabled.md index a3edd5dea9..8d8fb4c507 100644 --- a/.changeset/7687-combobox-option-disabled.md +++ b/.changeset/7687-combobox-option-disabled.md @@ -28,5 +28,15 @@ sibling select renderer, which already sets `disabled={opt.disabled}` on its `SelectItem`. Nothing else moves. The whole-control `disabled` prop (the one forwarded to the -trigger button) is untouched, `@object-ui/types` is untouched, and no new key is -introduced — this release only starts reading one that was already published. +trigger button) is untouched, no key is added to `@object-ui/types`, and no new +key is introduced — this release only starts reading one that was already +published. + +`@object-ui/types` is deliberately **not** given its own bump. The one file that +changes there is a test, `component-docs-disabled-inherited-7239.test.ts`: its +census over `content/docs/components` counts every documented `disabled?:` row, +and documenting the member adds a legitimate row that the ledger now claims as +INDEPENDENT (the shipped `ComboboxOption` declares `disabled` itself and does not +extend `BaseSchema`, so the narrow `boolean` spelling is correct for it). No +shipped type or value moves in that package, so there is no behaviour there to +version. diff --git a/packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.ts b/packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.ts index f8b03314aa..814cfb7980 100644 --- a/packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.ts +++ b/packages/types/src/__tests__/component-docs-disabled-inherited-7239.test.ts @@ -78,6 +78,33 @@ * That last pair is the point of the control: a failure that reddened the * independent rows too would mean the sweep was indiscriminate, not that the * inherited rows were wrong. + * + * ## Amendments + * + * The population is a ledger, not a constant: it moves when a page gains or + * loses a real `disabled` row. Each move is recorded here with its cause, so a + * later reader can tell a deliberate claim from a number someone bumped to get + * back to green. + * + * - **22 -> 23 rows, INDEPENDENT 8 -> 9 (objectui#7687).** `ComboboxOption` + * joins the independent table. Its `disabled` was always DECLARED by + * `packages/types/src/form.ts` and validated by `ComboboxOptionSchema`, but + * the combobox component never read it, so an option authored + * `disabled: true` rendered selectable; #7687 makes the component honour it + * and documents the member on `form/combobox.mdx`, which is the row this + * census then measured. It classifies INDEPENDENT on this file's own + * criterion, not by resemblance to its neighbours: the shipped + * `ComboboxOption` declares `disabled` itself and does NOT extend + * `BaseSchema`, so the narrow `boolean` is the correct spelling and the + * second INDEPENDENT assertion holds for it unchanged. + * + * ⚠️ Note for the class, since the #7687 card recorded the opposite: that + * page's `interface` block sits in a `plaintext` fence and the card + * concluded no CI gate could see it. True of the doc-type gates named + * above, and false overall — THIS census reads those `.mdx` files as text + * and caught the new row. A doc edit under `content/docs/components` that + * touches a `disabled` row is answerable to `packages/types`, so a run + * narrowed to the package the code fix lives in cannot see it. */ import { describe, expect, it } from 'vitest'; @@ -184,6 +211,8 @@ const INDEPENDENT = [ { page: 'basic/button-group.mdx', iface: 'ButtonGroupButton', shippedName: 'ButtonGroupButton' }, { page: 'disclosure/accordion.mdx', iface: 'AccordionItem', shippedName: 'AccordionItem' }, { page: 'disclosure/toggle-group.mdx', iface: 'ToggleGroupItem', shippedName: 'ToggleGroupItem' }, + // Added by objectui#7687, which made the member real — see `## Amendments`. + { page: 'form/combobox.mdx', iface: 'ComboboxOption', shippedName: 'ComboboxOption' }, { page: 'form/form.mdx', iface: 'FormField', shippedName: 'FormField' }, { page: 'form/radio-group.mdx', iface: 'RadioOption', shippedName: 'RadioOption' }, // Doc-local names; the shipped shape they illustrate is `MenuCommandItem`. @@ -246,7 +275,7 @@ describe('the two tables account for every documented `disabled` row (objectui#7 it('sees exactly the population this card measured', () => { expect({ rows: ROWS.length, inherited: INHERITED.length, independent: INDEPENDENT.length }).toEqual( - { rows: 22, inherited: 14, independent: 8 }, + { rows: 23, inherited: 14, independent: 9 }, ); }); });