Auto-hide labels of box fields nested in FormLayoutCustomField via context#699
Auto-hide labels of box fields nested in FormLayoutCustomField via context#699adamkudrna wants to merge 1 commit into
FormLayoutCustomField via context#699Conversation
d38a4ef to
abdf6f8
Compare
2a8871d to
3354820
Compare
|
3354820 to
cafc0b6
Compare
cafc0b6 to
588f31b
Compare
FormLayoutCustomField must hide their labelsFormLayoutCustomField via context
588f31b to
06bfda8
Compare
06bfda8 to
3fd563d
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a FormLayoutCustomField-scoped context signal so that “box” fields nested inside FormLayoutCustomField automatically hide their own labels, reducing grid/alignment conflicts and removing the need for repetitive isLabelVisible={false} in consuming code and tests.
Changes:
- Add
FormLayoutCustomFieldContextand provide it fromFormLayoutCustomField; update TextField/TextArea/SelectField/FileInputField to hide labels when the context is present. - Update FormLayout documentation to describe the nested-label behavior and adjust examples accordingly.
- Expand/adjust Playwright visual coverage (including new
labelWidth="100px"cases and new snapshots), and update stories/tests accordingly (including Button form-layout cases).
Reviewed changes
Copilot reviewed 32 out of 60 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Toggle/tests/Toggle.story.tsx | Adds a FormLayout label-width test story wrapper for Toggle. |
| src/components/Toggle/tests/Toggle.spec.tsx | Adds visual snapshot test for labelWidth="100px" scenario. |
| src/components/Toggle/tests/Toggle.spec.tsx-snapshots/Toggle-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new visual snapshot for Toggle label width. |
| src/components/TextField/TextField.jsx | Hides TextField label when inside FormLayoutCustomField via new context. |
| src/components/TextField/tests/TextField.story.tsx | Adds stories for FormLayout label width and FormLayoutCustomField label-hidden scenario. |
| src/components/TextField/tests/TextField.spec.tsx | Adds visual tests for label width and FormLayoutCustomField label-hidden scenario. |
| src/components/TextField/tests/TextField.spec.tsx-snapshots/TextField-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new TextField label width snapshot. |
| src/components/TextField/tests/TextField.spec.tsx-snapshots/TextField-formLayoutCustomField-visual-label-hidden-1-chromium-linux.png | Adds new TextField nested-label-hidden snapshot. |
| src/components/TextArea/TextArea.jsx | Hides TextArea label when inside FormLayoutCustomField via new context. |
| src/components/TextArea/tests/TextArea.story.tsx | Adds stories for FormLayout label width and FormLayoutCustomField label-hidden scenario. |
| src/components/TextArea/tests/TextArea.spec.tsx | Adds visual tests for label width and FormLayoutCustomField label-hidden scenario. |
| src/components/TextArea/tests/TextArea.spec.tsx-snapshots/TextArea-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new TextArea label width snapshot. |
| src/components/TextArea/tests/TextArea.spec.tsx-snapshots/TextArea-formLayoutCustomField-visual-label-hidden-1-chromium-linux.png | Adds new TextArea nested-label-hidden snapshot. |
| src/components/SelectField/SelectField.jsx | Hides SelectField label when inside FormLayoutCustomField via new context. |
| src/components/SelectField/tests/SelectField.story.tsx | Adds stories for FormLayout label width and FormLayoutCustomField label-hidden scenario. |
| src/components/SelectField/tests/SelectField.spec.tsx | Adds visual tests for label width and FormLayoutCustomField label-hidden scenario. |
| src/components/SelectField/tests/SelectField.spec.tsx-snapshots/SelectField-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new SelectField label width snapshot. |
| src/components/SelectField/tests/SelectField.spec.tsx-snapshots/SelectField-formLayoutCustomField-visual-label-hidden-1-chromium-linux.png | Adds new SelectField nested-label-hidden snapshot. |
| src/components/Radio/tests/Radio.story.tsx | Adds a FormLayout label-width test story wrapper for Radio. |
| src/components/Radio/tests/Radio.spec.tsx | Adds visual snapshot test for labelWidth="100px" scenario. |
| src/components/Radio/tests/Radio.spec.tsx-snapshots/Radio-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new Radio label width snapshot. |
| src/components/FormLayout/README.md | Documents nested-field label auto-hiding and updates examples accordingly. |
| src/components/FormLayout/index.js | Exports FormLayoutCustomFieldContext from the FormLayout barrel. |
| src/components/FormLayout/FormLayoutCustomFieldContext.js | Introduces new context used to signal “inside custom field”. |
| src/components/FormLayout/FormLayoutCustomField.jsx | Provides FormLayoutCustomFieldContext to descendants. |
| src/components/FormLayout/tests/FormLayoutCustomField.story.tsx | Minor import reordering for tests/stories. |
| src/components/FormLayout/tests/FormLayoutCustomField.spec.tsx-snapshots/FormLayoutCustomField-base-visual-labelVisible-boolean-true-fieldLayout-string-horizontal-1-chromium-linux.png | Updates/introduces snapshot for horizontal custom-field visuals. |
| src/components/FormLayout/tests/FormLayout.story.tsx | Adds Button to FormLayout test story composition. |
| src/components/FormLayout/tests/FormLayout.spec.tsx-snapshots/FormLayout-base-visual-labelWidth-string-auto-1-chromium-linux.png | Updates snapshot due to story/layout changes. |
| src/components/FormLayout/tests/_propTests/labelWidthPropTest.ts | Renames label width test case from 30px to 100px and updates props. |
| src/components/FormLayout/tests/_propTests/customFieldTests/customFieldValidationStatesPropTest.ts | Removes nested-field isLabelVisible wiring (now context-driven). |
| src/components/FormLayout/tests/_propTests/customFieldTests/customFieldValidationStatesDisabledPropTest.ts | Removes nested-field isLabelVisible wiring (now context-driven). |
| src/components/FormLayout/tests/_propTests/customFieldTests/customFieldRequiredPropTest.ts | Removes nested-field isLabelVisible wiring (now context-driven). |
| src/components/FormLayout/tests/_propTests/customFieldTests/customFieldLabelAlignmentPropTest.ts | Removes label-visibility variants; leaves simplified cases. |
| src/components/FormLayout/tests/_propTests/customFieldTests/customFieldFullWidthPropTest.ts | Removes nested-field isLabelVisible wiring (now context-driven). |
| src/components/FileInputField/FileInputField.jsx | Hides FileInputField label when inside FormLayoutCustomField via new context. |
| src/components/FileInputField/tests/FileInputField.story.tsx | Adds stories for FormLayout label width and FormLayoutCustomField label-hidden scenario. |
| src/components/FileInputField/tests/FileInputField.spec.tsx | Adds visual tests for label width and FormLayoutCustomField label-hidden scenario. |
| src/components/FileInputField/tests/FileInputField.spec.tsx-snapshots/FileInputField-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new FileInputField label width snapshot. |
| src/components/FileInputField/tests/FileInputField.spec.tsx-snapshots/FileInputField-formLayoutCustomField-visual-label-hidden-1-chromium-linux.png | Adds new FileInputField nested-label-hidden snapshot. |
| src/components/CheckboxField/tests/CheckboxField.story.tsx | Adds a FormLayout label-width test story wrapper for CheckboxField. |
| src/components/CheckboxField/tests/CheckboxField.spec.tsx | Adds visual snapshot test for labelWidth="100px" scenario. |
| src/components/CheckboxField/tests/CheckboxField.spec.tsx-snapshots/CheckboxField-formLayout-visual-labelWidth-string-100px-1-chromium-linux.png | Adds new CheckboxField label width snapshot. |
| src/components/Button/tests/Button.story.tsx | Adds context-driven FormLayout tests for Button; updates FormLayout stories to include TextField context. |
| src/components/Button/tests/Button.spec.tsx | Adds FormLayout context-based visual tests using propTests.layoutPropTest. |
| src/components/Button/tests/Button.spec.tsx-snapshots/Button-formLayout-visual-layout-string-vertical-1-chromium-linux.png | Adds snapshot for Button layout via context (vertical). |
| src/components/Button/tests/Button.spec.tsx-snapshots/Button-formLayout-visual-layout-string-horizontal-1-chromium-linux.png | Adds snapshot for Button layout via context (horizontal). |
| src/components/Button/tests/Button.spec.tsx-snapshots/Button-formLayout-visual-vertical-1-chromium-linux.png | Updates/adds vertical FormLayout button snapshot. |
| src/components/Button/tests/Button.spec.tsx-snapshots/Button-formLayout-visual-horizontal-1-chromium-linux.png | Updates/adds horizontal FormLayout button snapshot. |
| src/components/Button/tests/Button.spec.tsx-snapshots/Button-formLayout-visual-vertical-1-chromium-linux.png | Adds/updates snapshot coverage for vertical layout variant. |
Comments suppressed due to low confidence (1)
src/components/FormLayout/tests/_propTests/customFieldTests/customFieldLabelAlignmentPropTest.ts:32
customFieldLabelAlignmentPropTeststill uses test names likelabelVisible:boolean=false ..., but the test case no longer setsisLabelVisible: false(and in fact the nested field label is now always hidden viaFormLayoutCustomFieldContext). This makes the test name misleading and harder to interpret; consider renaming these cases to reflect the new behavior (e.g.nestedLabel:hidden ...) or reintroducing the prop if you still want to cover it explicitly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /> | ||
| ); | ||
|
|
||
| export const RadioForFormLayoutLabelWidthTests = () => ( |
There was a problem hiding this comment.
General question.
Why there two types of form layout story components? One uses FormLayoutContext and you introduced FormLayout. Why to not we extend RadioForFormLayoutTests with labelWidth (as it is done with layout)?
It would make sense to me to have just one component for such tests as everything can be simplified then. Including just simple propTest instead of writing specific test case in spec file.
We do test component here, so setting FormLayoutContext is more correct. Otherwise you test composition of two components.
There was a problem hiding this comment.
Agreed for layout/fieldLayout — that is purely context-driven (FormLayoutContext → { layout }), so RadioForFormLayoutTests wrapping in FormLayoutContext.Provider is the right, more isolated approach.
labelWidth is different: it is CSS-only, set as grid-template-columns/--rui-custom-label-width on the real FormLayout root, and consumed by fields via CSS Grid subgrid. That only resolves with a real grid ancestor in the DOM, so it cannot be faked through context. RadioForFormLayoutLabelWidthTests wrapping in a real <FormLayout> is required for that case, not composition-testing overreach.
…context
Creates `FormLayoutCustomFieldContext` and has `FormLayoutCustomField` provide
it to its children. `TextField`, `TextArea`, `SelectField`, and
`FileInputField` consume the context to automatically hide their labels when
rendered inside a `FormLayoutCustomField`, making the behavior a hard rule
rather than a recommendation.
Updates the `FormLayout` README to reflect the automatic behavior and removes
all now-redundant `isLabelVisible={false}` from code examples. Removes the
prop from `FormLayoutCustomField` test fixtures and the `FormLayout` story
accordingly.
Also adds per-component `formLayout` test coverage for `labelWidth` and the
new `formLayoutCustomField` visual tests to `TextField`, `TextArea`,
`SelectField`, `FileInputField`, `Button`, `CheckboxField`, `Radio`, and
`Toggle`.
3fd563d to
febdea1
Compare
| return ( | ||
| <FormLayoutContext.Provider | ||
| value={values} | ||
| > |
There was a problem hiding this comment.
I think we should add here <TextField label="Text field" />. Otherwise the test snapshot just shows the button and it does not test the layout at all.
Or is omitted on purpose and I'm missing something? Other components have a test named …visual-label-hidden… so maybe it is just a misleading name?
There was a problem hiding this comment.
Are we dropping the snapshots on purpose? Why?
Summary
FormLayoutCustomFieldContextand provides it fromFormLayoutCustomField, soTextField,TextArea,SelectField, andFileInputFieldautomatically hide their own label when nested insideFormLayoutCustomField— no more manualisLabelVisible={false}needed.This is a breaking change: any nested field that previously showed its
own label will now always have it hidden, since
FormLayoutforceshorizontal layout on all descendants and a visible nested label would
conflict with the grid regardless of
fieldLayout.FormLayoutREADMEdocumenting the automatic behavior.
labelWidth="100px"visual test coverage (via a realFormLayoutwrapper) for
Button,CheckboxField,FileInputField,Radio,SelectField,TextArea,TextField, andToggle, and renames theprevious
30pxFormLayout label-width test case to the more representative100px.Buttonto theFormLayoutstory.isLabelVisible={false}wiring from custom-fieldprop tests since label-hiding is now automatic.