Skip to content

Navigation Menu

Sign in
Sign up

Commit d043454

Browse files
fix(docsite): bridge Selector's onChange preview even without a seeded value (#6035)
PR #5909 seeded `options`/`label` playground defaults for Selector so its Properties preview stops showing the "missing required props" placeholder, but did not seed `value` (correctly: Selector's own contract requires "closed with no value" to be a valid representative state, FR1 in Selector.spec.md). That exposed a latent bug in `buildRuntimePreviewState`: it only bridges a controlled callback back into playground state when its target prop already has a value in `state`, so Selector's `onChange` was never wired up and the live preview stayed frozen on the placeholder no matter what option a person clicked. Fix generalizes the bridge instead of papering over Selector specifically: the literal `value`/`onChange` pair — a component's one primary controlled value — now bridges whenever `target` names a real prop, even an optional one with no seeded default. Secondary paired callbacks (onIndexChange, onPageSizeChange, onOpenChange, ...) keep the original opt-in behavior, preserving each preview's chosen representative starting state (verified by the existing Lightbox gallery-index and overlay tests, unchanged and still passing). Added a focused regression test reproducing the exact reported scenario: Selector with options seeded but no value, selecting Orange must update preview state. Testing: - New test: bridges Selector onChange even though its optional value prop is not seeded (red before the fix, green after) - Full docsite suite: 34 files, 475 tests passing - tsc --noEmit: no new errors (pre-existing theme-package /built errors are unrelated to this worktree's unbuilt theme packages) - eslint on both changed files: clean - check:package-boundaries: clean Excludes all work from #5963 (unrelated theme appearance-nesting tokens); touches only docsite preview infrastructure and its test.
1 parent 75a4d1d commit d043454

2 files changed

Lines changed: 46 additions & 8 deletions

File tree

‎apps/docsite/src/__tests__/component-preview-state.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,33 @@ describe('component detail preview state', () => {
444444
expect(onPropChange).toHaveBeenCalledWith('value', 42);
445445
});
446446

447+
it('bridges Selector onChange even though its optional value prop is not seeded (#5909 follow-up)', () => {
448+
const knobs = pickPrimaryProps('Selector', [
449+
prop({name: 'label', type: 'string', required: true}),
450+
prop({name: 'options', type: 'SelectorOption[]', required: true}),
451+
prop({name: 'value', type: 'string'}),
452+
prop({name: 'onChange', type: '(value: string) => void'}),
453+
]);
454+
455+
const state = buildInitialState(knobs, {
456+
defaults: {
457+
label: 'Fruit',
458+
options: [
459+
{value: 'apple', label: 'Apple'},
460+
{value: 'orange', label: 'Orange'},
461+
],
462+
},
463+
});
464+
expect(state.value).toBeUndefined();
465+
expect(getMissingRequiredProps(knobs, state)).toEqual([]);
466+
467+
const onPropChange = vi.fn();
468+
const runtimeState = buildRuntimePreviewState(state, onPropChange, {knobs});
469+
470+
(runtimeState.onChange as (value: string) => void)('orange');
471+
expect(onPropChange).toHaveBeenCalledWith('value', 'orange');
472+
});
473+
447474
it('bridges a Tokenizer removal back to its controlled value', () => {
448475
const knobs = pickPrimaryProps('Tokenizer', [
449476
prop({name: 'label', type: 'string', required: true}),

‎apps/docsite/src/components/component-detail/interactiveState.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -293,26 +293,35 @@ function getCallbackTargetProp(type: string): string | null {
293293
/**
294294
* The state prop a change handler writes back to, or `null` when the preview
295295
* cannot tell. Prefers the first parameter's name; for `onChange` falls back
296-
* to the controlled `value` prop, whose payload it carries by convention.
296+
* to the controlled `value` prop, whose payload it carries by convention. The
297+
* primary `onChange` pair may target an optional, unseeded value prop; secondary
298+
* change handlers still require their target to be present in preview state.
297299
*/
298300
function resolveChangeTarget(
299301
name: string,
300302
type: string,
301303
state: Record<string, unknown>,
304+
knownProps: ReadonlySet<string>,
302305
): string | null {
303306
const named = getCallbackTargetProp(type);
304-
if (named != null && named in state) {
307+
if (
308+
named != null &&
309+
(named in state || (name === 'onChange' && knownProps.has(named)))
310+
) {
305311
return named;
306312
}
307-
return name === 'onChange' && 'value' in state ? 'value' : null;
313+
return name === 'onChange' && ('value' in state || knownProps.has('value'))
314+
? 'value'
315+
: null;
308316
}
309317

310318
/**
311319
* Wires controlled-component change handlers back into playground state so the
312320
* preview reflects interaction (clicking a Pagination page, etc.). A callback
313-
* whose first parameter names a value prop in `state` (page/onChange,
314-
* value/onChange, pageSize/onPageSizeChange) replaces its noop with one that
315-
* updates that prop. isOpen/onOpenChange stays gated behind canControlOpenState.
321+
* whose first parameter names a value prop in preview state (page/onChange,
322+
* pageSize/onPageSizeChange), or the primary `value` prop for literal
323+
* `onChange`, replaces its noop with one that updates that prop.
324+
* isOpen/onOpenChange stays gated behind canControlOpenState.
316325
*
317326
* `onChange` is the exception to the name match: it conventionally documents
318327
* its first parameter after the payload it carries (`checked`, `items`,
@@ -330,10 +339,12 @@ export function buildRuntimePreviewState(
330339
return state;
331340
}
332341

342+
const knobs = options?.knobs ?? [];
343+
const knownProps = new Set(knobs.map(knob => knob.row.name));
333344
const next: Record<string, unknown> = {...state};
334345
let changed = false;
335346

336-
for (const {row, control} of options?.knobs??[]) {
347+
for (const {row, control} of knobs) {
337348
if (control.kind !== 'callback') {
338349
continue;
339350
}
@@ -342,7 +353,7 @@ export function buildRuntimePreviewState(
342353
if (!/^on[A-Z].*Change$/.test(row.name) && row.name !== 'onChange') {
343354
continue;
344355
}
345-
const target = resolveChangeTarget(row.name, row.type, state);
356+
const target = resolveChangeTarget(row.name, row.type, state,knownProps);
346357
if (target == null) {
347358
continue;
348359
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /