feat: Add inlineLabelText to input - #4956
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4956 +/- ##
=======================================
Coverage 97.66% 97.66%
=======================================
Files 959 959
Lines 31345 31354 +9
Branches 11579 11582 +3
=======================================
+ Hits 30614 30623 +9
Misses 724 724
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| expect(element).not.toHaveAttribute('aria-invalid'); | ||
| }); | ||
|
|
||
| test('renders inline label when __inlineLabelText is provided', () => { |
There was a problem hiding this comment.
Consolidated input inline label tests into the new file src/input/__tests__/inline-label.test.tsx
| style?: InputProps.Style; | ||
|
|
||
| /** | ||
| * Adds a small label inline with the input for saving vertical space in the UI. |
There was a problem hiding this comment.
Based on Select API doc text:
components/src/select/interfaces.ts
Line 185 in dec0201
| /> | ||
| ); | ||
|
|
||
| const inputWithLabel = __inlineLabelText ? ( |
There was a problem hiding this comment.
To support the combination of inline label + suffix/prefix, the input with suffix/prefix needs to be rendered inside and a container with the label outside, not the other way around. This is what these changes do.
There was a problem hiding this comment.
🟡 Changes recommended
The inline <label htmlFor> can become desynchronized from the native input id when nativeInputAttributes.id is used, which breaks label association.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a new public inlineLabelText API for the Input component to support inline labels (useful for space-constrained UIs), and updates internal consumers and tests accordingly.
Changes:
- Add
inlineLabelText?: stringtoInputPropsand plumb it throughInput→InternalInput. - Update
InternalInputrendering to support an inline<label>(including generatedcontrolIdwhen needed) and add a disabled inline-label style. - Add dedicated unit coverage + a permutations page for visual/regression review; update Pagination to use the new prop.
File summaries
| File | Description |
|---|---|
| src/test-utils/dom/input/index.ts | Adds findInlineLabel() test-utils helper for the new inline label element. |
| src/pagination/internal.tsx | Switches jump-to-page input to use the new public inlineLabelText prop. |
| src/input/styles.scss | Adds disabled styling hook for the inline label. |
| src/input/internal.tsx | Implements inline label rendering, generated IDs, and adjusts root vs container prop application. |
| src/input/interfaces.ts | Exposes inlineLabelText on the public InputProps API with documentation. |
| src/input/index.tsx | Passes inlineLabelText through to the internal implementation. |
| src/input/tests/internal.test.tsx | Removes legacy tests tied to the private __inlineLabelText prop. |
| src/input/tests/inline-label.test.tsx | Adds unit tests for inlineLabelText behavior and ID/label association. |
| src/tests/snapshot-tests/snapshots/test-utils-selectors.test.tsx.snap | Updates selector snapshots for the new inline label selector. |
| src/tests/snapshot-tests/snapshots/documenter.test.ts.snap | Updates generated API/test-utils documentation snapshots for the new prop/helper. |
| pages/input/inline-label-permutations.page.tsx | Adds a permutations page to manually verify inline-label rendering combinations. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The current controlId precedence can silently break FormField label-to-input association and suppress override warnings when both controlId/context and nativeInputAttributes.id are provided.
Review details
Suppressed comments (1)
src/input/internal.tsx:150
controlIdcurrently prioritizesnativeInputAttributes.idover the officialcontrolId(and FormField context) value. This makescontrolIdunexpectedly ignored when both are provided and also suppressesWithNativeAttributes' override warning, which can hide a broken FormField<label htmlFor>→ inputidassociation when the IDs differ. Consider usingnativeInputAttributes.idonly as a fallback whencontrolIdis otherwise missing, primarily to support the inline-label case.
// When an inline label is rendered, the native input must have an id so the
// label's htmlFor can reference it. Fall back to a generated id if none was provided.
const generatedControlId = useUniqueId('input');
const controlId =
nativeInputAttributes?.id ?? controlIdFromFormFieldContext ?? (inlineLabelText ? generatedControlId : undefined);
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
…#4949) Co-authored-by: Gethin Webster <gethinw@amazon.de>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Although the input and label would always be in sync, override warnings could have been silenced by |
Description
Required for control group feature (doc:
dwYUc9i8X0rP, draft PR: #4969)How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.