Skip to content

Commit ad17ed8

Browse files
os-zhuangclaude
andcommitted
fix(form): keep the fieldTabs schema keys off the DOM, and make tab position work
Two follow-ups found by running the tabbed form in a real browser (console preview gallery): - `fieldTabs` / `defaultFieldTab` / `fieldTabsPosition` bled through `...formProps` onto the <form> element ("React does not recognize the `fieldTabs` prop on a DOM element"), the same leak the neighbouring destructure already guards `objectName` / `onDirtyChange` / `fields` against. Added to that list, with a regression test that spreads the form node at the top level the way the SDUI dispatch does. - `fieldTabsPosition: 'bottom' | 'right'` positioned the strip with `order-last`, which does nothing unless the parent is a flex container — the root was only flex in the vertical case (a quirk carried over verbatim from TabbedForm). The Tabs root is now always flex (`flex-col` when horizontal), the strip keeps its content size via `self-start`, and `bottom` swaps its margin to `mt-4`. Re-verified in the browser: single form, panels force-mounted with the inactive ones computed `display: none`, strip content-width above the panel, and no console errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5af687e commit ad17ed8

2 files changed

Lines changed: 34 additions & 4 deletions

File tree

packages/components/src/renderers/form/__tests__/form-field-tabs.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,27 @@ describe('form renderer — fieldTabs (#2959)', () => {
138138
expect(screen.getByTestId('form-tab-panel:detail')).toHaveAttribute('data-state', 'active');
139139
});
140140

141+
it('keeps the tab schema keys off the <form> DOM element', () => {
142+
// Callers/the SDUI dispatch spread the whole form node at the top level too,
143+
// so a FormSchema key that isn't consumed leaks onto the DOM and React logs
144+
// "does not recognize the `fieldTabs` prop on a DOM element".
145+
const Form = ComponentRegistry.get('form')!;
146+
const { container } = render(
147+
<Form
148+
schema={{ type: 'form', fields: FIELDS, fieldTabs: TABS }}
149+
// The top-level spread that reproduces the leak.
150+
fields={FIELDS}
151+
fieldTabs={TABS}
152+
defaultFieldTab="detail"
153+
fieldTabsPosition="top"
154+
/>,
155+
);
156+
const form = container.querySelector('form')!;
157+
for (const attr of ['fieldtabs', 'defaultfieldtab', 'fieldtabsposition']) {
158+
expect(form.hasAttribute(attr)).toBe(false);
159+
}
160+
});
161+
141162
it('renders fields no tab claims instead of dropping them', () => {
142163
const { container } = renderTabbedForm({
143164
fieldTabs: [

packages/components/src/renderers/form/form.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,6 +1151,9 @@ ComponentRegistry.register('form',
11511151
columns: _columns,
11521152
validationMode: _validationMode,
11531153
disabled: _disabledProp,
1154+
fieldTabs: _fieldTabs,
1155+
defaultFieldTab: _defaultFieldTab,
1156+
fieldTabsPosition: _fieldTabsPosition,
11541157
...formProps
11551158
} = props;
11561159

@@ -1195,13 +1198,19 @@ ComponentRegistry.register('form',
11951198
value={activeFieldTab}
11961199
onValueChange={setPickedFieldTab}
11971200
orientation={isVerticalFieldTabs ? 'vertical' : 'horizontal'}
1198-
className={cn('w-full', isVerticalFieldTabs && 'flex gap-4')}
1201+
// Always a flex container: `order-last` on the strip is what
1202+
// puts it after the panels for `bottom`/`right`, and `order`
1203+
// only applies to flex children.
1204+
className={cn('flex w-full', isVerticalFieldTabs ? 'gap-4' : 'flex-col')}
11991205
>
12001206
<TabsList
12011207
className={cn(
1202-
'mb-4',
1203-
isVerticalFieldTabs && 'h-auto flex-col',
1204-
(fieldTabsPosition === 'bottom' || fieldTabsPosition === 'right') && 'order-last',
1208+
// `self-start` keeps the strip its content's size instead of
1209+
// being stretched by the flex container above.
1210+
'self-start',
1211+
isVerticalFieldTabs
1212+
? cn('h-auto flex-col', fieldTabsPosition === 'right' && 'order-last')
1213+
: fieldTabsPosition === 'bottom' ? 'order-last mt-4' : 'mb-4',
12051214
)}
12061215
>
12071216
{fieldTabGroups.map((group) => (

0 commit comments

Comments
 (0)