diff --git a/apps/docsite/src/__tests__/playground-number-control.test.ts b/apps/docsite/src/__tests__/playground-number-control.test.ts new file mode 100644 index 000000000000..e8a20eceee11 --- /dev/null +++ b/apps/docsite/src/__tests__/playground-number-control.test.ts @@ -0,0 +1,80 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file playground-number-control.test.ts + * + * Regression coverage for the playground "number" knob (issue: a Card with an + * unset \`maxWidth\` showed \`0\` in the editor, and stepping 0 -> 1 -> 0 left a + * literal \`maxWidth={0}\` in the source — collapsing sizing props with no path + * back to the original unset render). + * + * Two invariants are pinned here: + * 1. \`SizeValue\` (and bare \`number\`) resolve to the \`number\` control, so size + * props get a numeric knob in the first place. + * 2. Clearing a numeric prop in the Properties popover removes the attribute + * from source (returning it to unset) instead of writing a literal value. + * This mirrors the \`commit('number', null)\` branch in PropertyEditor. + */ + +import {describe, expect, it} from 'vitest'; +import {parsePropType} from '../components/component-detail/parsePropType'; +import { + analyzeCode, + formatAttr, + removeAttribute, + setAttribute, + type InstanceInfo, +} from '../app/playground/propertyEditor/componentInstances'; + +function firstInstance(code: string): InstanceInfo { + const instances = analyzeCode(code); + expect(instances).not.toBeNull(); + expect(instances!.length).toBeGreaterThan(0); + return instances![0]; +} + +/** + * Mirror of PropertyEditor's number commit handler: a null value clears the + * prop, any concrete number writes a literal. + */ +function commitNumber( + code: string, + instance: InstanceInfo, + name: string, + value: number | null, +): string { + return value == null + ? removeAttribute(code, instance, name) + : setAttribute(code, instance, name, formatAttr(name, 'number', value)); +} + +describe('playground number control', () => { + it('maps SizeValue and number to the number control', () => { + expect(parsePropType('SizeValue', 'maxWidth')).toEqual({kind: 'number'}); + expect(parsePropType('number', 'ratio')).toEqual({kind: 'number'}); + }); + + it('clearing a numeric prop removes it from source (no literal left behind)', () => { + const base = 'content'; + + // Step the value up to 1, as the stepper would. + const withMaxWidth = commitNumber(base, firstInstance(base), 'maxWidth', 1); + expect(withMaxWidth).toBe('content'); + + // Clearing returns the prop to unset rather than writing maxWidth={0}. + const cleared = commitNumber( + withMaxWidth, + firstInstance(withMaxWidth), + 'maxWidth', + null, + ); + expect(cleared).toBe(base); + expect(cleared).not.toContain('maxWidth'); + }); + + it('explicitly setting 0 still writes a literal (0 is a valid value)', () => { + const base = 'content'; + const withZero = commitNumber(base, firstInstance(base), 'maxWidth', 0); + expect(withZero).toBe('content'); + }); +}); diff --git a/apps/docsite/src/app/playground/propertyEditor/PropertyEditor.tsx b/apps/docsite/src/app/playground/propertyEditor/PropertyEditor.tsx index a20a4320c7d0..9defc0a18056 100644 --- a/apps/docsite/src/app/playground/propertyEditor/PropertyEditor.tsx +++ b/apps/docsite/src/app/playground/propertyEditor/PropertyEditor.tsx @@ -113,7 +113,7 @@ function PropRow({prop, instance, code, onCodeChange}: PropRowProps) { const commit = ( kind: 'boolean' | 'string' | 'number' | 'enum', - value: string | number | boolean, + value: string | number | boolean | null, ) => { let next: string; if (kind === 'boolean') { @@ -133,30 +133,45 @@ function PropRow({prop, instance, code, onCodeChange}: PropRowProps) { code, instance, prop.name, - formatAttr(prop.name, 'string', value), + formatAttr(prop.name, 'string', String(value)), ); } else if (kind === 'number') { - next = setAttribute( - code, - instance, - prop.name, - formatAttr(prop.name, 'number', value), - ); + // A cleared number returns the prop to unset rather than writing a + // literal (e.g. maxWidth={0}, which collapses sizing props). + next = + value == null + ? removeAttribute(code, instance, prop.name) + : setAttribute( + code, + instance, + prop.name, + formatAttr(prop.name, 'number', value), + ); } else { - const numeric = - typeof value === 'number' || NUMERIC_RE.test(String(value)); - const attrKind = - typeof value === 'boolean' ? 'boolean' : numeric ? 'number' : 'string'; - next = setAttribute( - code, - instance, - prop.name, - formatAttr( + // Enum commits always carry a concrete option value; treat a null (only + // emitted by the number control) as clearing the attribute. + if (value == null) { + next = removeAttribute(code, instance, prop.name); + } else { + const numeric = + typeof value === 'number' || NUMERIC_RE.test(String(value)); + const attrKind = + typeof value === 'boolean' + ? 'boolean' + : numeric + ? 'number' + : 'string'; + next = setAttribute( + code, + instance, prop.name, - attrKind, - numeric && typeof value !== 'number' ? Number(value) : value, - ), - ); + formatAttr( + prop.name, + attrKind, + numeric && typeof value !== 'number' ? Number(value) : value, + ), + ); + } } onCodeChange(next); }; @@ -208,13 +223,18 @@ function PropRow({prop, instance, code, onCodeChange}: PropRowProps) { ); } else { const def = coerceDefault(prop.default, control) as number | undefined; - const value = typeof attr?.value === 'number' ? attr.value : (def ?? 0); + // Reflect the actual source state: a prop that isn't set in code (and has + // no default) is unset, not `0`. Showing a real `0` here would let a size + // prop like maxWidth render `max-width: 0` with no way back to unset. + const value = typeof attr?.value === 'number' ? attr.value : (def ?? null); controlEl = ( commit('number', next)} + placeholder={prop.required ? undefined : 'unset'} + hasClear={!prop.required} + onChange={(next: number | null) => commit('number', next)} xstyle={s.inputControl} /> ); diff --git a/apps/docsite/src/components/component-detail/PlaygroundPropsTable.tsx b/apps/docsite/src/components/component-detail/PlaygroundPropsTable.tsx index da87d562ab6b..35a246b26eb8 100644 --- a/apps/docsite/src/components/component-detail/PlaygroundPropsTable.tsx +++ b/apps/docsite/src/components/component-detail/PlaygroundPropsTable.tsx @@ -429,11 +429,18 @@ function InlineControl({ /> ); case 'number': + // An optional prop with no default is genuinely unset — show an empty + // input (not a fabricated `0`, which misrepresents the live state and, + // for size props like maxWidth, has no path back to "unset"). `hasClear` + // lets the user return to the original unset render. Required numbers + // keep their generated fallback and stay non-clearable. return ( onChange(next)} + value={typeof value === 'number' ? value : null} + placeholder={prop.required ? undefined : 'unset'} + hasClear={!prop.required} + onChange={(next: number | null) => onChange(next ?? undefined)} /> ); case 'element':