Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions packages/components/text-inputs/src/TextField/TextField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,14 +339,16 @@ const TextField = forwardRef(
}, [maxLength, validation, isRequiredAndEmpty, inputValue]);

const hasIcon = iconName || secondaryIconName;
const shouldShowExtraText =
showCharCount || (validation && validation.text) || (isRequiredAndEmpty && requiredErrorText);
const hasValidationText = !!((validation && validation.text) || (isRequiredAndEmpty && requiredErrorText));
const shouldShowExtraText = showCharCount || hasValidationText;
const isSecondary = secondaryIconName === currentStateIconName;
const isPrimary = iconName === currentStateIconName;
const shouldFocusOnPrimaryIcon =
(onIconClick !== NOOP || iconLabel || iconTooltipContent) && inputValue && iconName.length && isPrimary;
const shouldFocusOnSecondaryIcon = (secondaryIconName || secondaryTooltipContent) && isSecondary && !!inputValue;
const allowExceedingMaxLengthTextId = allowExceedingMaxLength ? `${id}-allow-exceeding-max-length-text` : undefined;
const validationTextId = hasValidationText ? `${id}-validation-text` : undefined;
const describedBy = [validationTextId, allowExceedingMaxLengthTextId].filter(Boolean).join(" ") || undefined;
Comment on lines +350 to +351

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Screen readers can announce wrong errors 🐞 Bug ≡ Correctness

validationTextId is derived from the shared default id value of "input", so every ID-less
instance renders the same validation target and description reference. When two validation-bearing
TextField instances omit id, the second input cannot be uniquely associated with its own message
and may resolve to the first field's error instead.
Agent Prompt
## Issue description
Multiple `TextField` instances that omit `id` generate duplicate validation-text IDs, preventing each input from being uniquely associated with its own message.

## Issue Context
The public `id` prop remains optional and defaults to the literal `"input"`. Generate a stable per-instance fallback identifier compatible with the package's supported React versions, use it consistently for the input, label, test IDs, and derived validation IDs, and add coverage for two ID-less validation-bearing fields.

## Fix Focus Areas
- packages/components/text-inputs/src/TextField/TextField.tsx[224-224]
- packages/components/text-inputs/src/TextField/TextField.tsx[349-351]
- packages/components/text-inputs/src/TextField/TextField.tsx[377-409]
- packages/components/text-inputs/src/TextField/TextField.tsx[476-480]
- packages/components/text-inputs/src/TextField/__tests__/TextField.test.tsx[342-396]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


useEffect(() => {
if (!inputRef?.current || !autoFocus) {
Expand Down Expand Up @@ -401,10 +403,10 @@ const TextField = forwardRef(
role={searchResultsContainerId && "combobox"} // For voice reader
aria-label={inputAriaLabel || placeholder}
aria-invalid={(validation && validation.status === "error") || isRequiredAndEmpty}
aria-owns={searchResultsContainerId}
aria-activedescendant={activeDescendant}
aria-owns={searchResultsContainerId || undefined}
aria-activedescendant={activeDescendant || undefined}
aria-required={required}
aria-describedby={allowExceedingMaxLengthTextId}
aria-describedby={describedBy}
required={required}
tabIndex={tabIndex}
dir={dir}
Expand Down Expand Up @@ -473,8 +475,8 @@ const TextField = forwardRef(
</div>
{shouldShowExtraText && (
<Text type="text2" color="secondary" className={cx(styles.subTextContainer)}>
{((validation && validation.text) || (isRequiredAndEmpty && requiredErrorText)) && (
<span className={cx(styles.subTextContainerStatus)}>
{hasValidationText && (
<span id={validationTextId} className={cx(styles.subTextContainerStatus)}>
{isRequiredAndEmpty ? requiredErrorText : validation.text}
</span>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,62 @@ describe("TextField Tests", () => {
expect(input.value).toBe(value.trim());
});

describe("validation text association", () => {
it("should point aria-describedby at the rendered validation text element", () => {
const { container } = render(
<TextField onChange={onChangeStub} id="described" validation={{ status: "error", text: "Invalid Email" }} />
);
const input = container.querySelector("#described");

expect(input.getAttribute("aria-describedby")).toBe("described-validation-text");
expect(container.querySelector("#described-validation-text").textContent).toBe("Invalid Email");
});

it("should associate the required error text once the field is blurred while empty", () => {
const { container } = render(
<TextField onChange={onChangeStub} id="described" required requiredErrorText="This field is required" />
);
const input = container.querySelector("#described");
act(() => {
fireEvent.blur(input, { target: { value: "" } });
});

expect(input.getAttribute("aria-describedby")).toBe("described-validation-text");
expect(container.querySelector("#described-validation-text").textContent).toBe("This field is required");
});

it("should compose the validation text id with the max length hint id", () => {
const { container } = render(
<TextField
onChange={onChangeStub}
id="described"
showCharCount
maxLength={5}
allowExceedingMaxLength
validation={{ status: "error", text: "Invalid Email" }}
/>
);

expect(container.querySelector("#described").getAttribute("aria-describedby")).toBe(
"described-validation-text described-allow-exceeding-max-length-text"
);
});

it("should not set aria-describedby when there is no validation text", () => {
const { container } = render(<TextField onChange={onChangeStub} id="described" />);

expect(container.querySelector("#described").hasAttribute("aria-describedby")).toBe(false);
});

it("should omit aria-owns and aria-activedescendant when they are empty", () => {
const { container } = render(<TextField onChange={onChangeStub} id="described" />);
const input = container.querySelector("#described");

expect(input.hasAttribute("aria-owns")).toBe(false);
expect(input.hasAttribute("aria-activedescendant")).toBe(false);
});
});

describe("controlled", () => {
it("should call onChange with the new value when controlled is true", () => {
const handleChange = vi.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ exports[`TextField renders correctly > when disabled 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -54,10 +52,8 @@ exports[`TextField renders correctly > when loading 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -124,10 +120,8 @@ exports[`TextField renders correctly > when readonly 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input readOnly"
Expand Down Expand Up @@ -165,10 +159,8 @@ exports[`TextField renders correctly > when required 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={true}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -206,10 +198,8 @@ exports[`TextField renders correctly > with another type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -247,10 +237,8 @@ exports[`TextField renders correctly > with className 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="testClassName input"
Expand Down Expand Up @@ -288,10 +276,8 @@ exports[`TextField renders correctly > with date type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -329,10 +315,8 @@ exports[`TextField renders correctly > with date-time type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -370,10 +354,8 @@ exports[`TextField renders correctly > with email type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -411,10 +393,8 @@ exports[`TextField renders correctly > with icon 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input inputHasIcon"
Expand Down Expand Up @@ -472,10 +452,8 @@ exports[`TextField renders correctly > with iconLabel and secondaryIconLabel 1`]
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -513,10 +491,8 @@ exports[`TextField renders correctly > with id 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -554,10 +530,8 @@ exports[`TextField renders correctly > with labelIconName 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -595,10 +569,8 @@ exports[`TextField renders correctly > with large size 1`] = `
className="inputWrapper wrapperSizeLarge"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -636,10 +608,8 @@ exports[`TextField renders correctly > with placeholder 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label="placeholder"
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -677,10 +647,8 @@ exports[`TextField renders correctly > with role 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -718,10 +686,8 @@ exports[`TextField renders correctly > with secondaryIconName 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input inputHasIcon"
Expand Down Expand Up @@ -779,10 +745,8 @@ exports[`TextField renders correctly > with showCharCount 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -838,10 +802,8 @@ exports[`TextField renders correctly > with tel type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -879,10 +841,8 @@ exports[`TextField renders correctly > with url type 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -920,10 +880,9 @@ exports[`TextField renders correctly > with validation 1`] = `
className="inputWrapper wrapperSizeSmall inputErrorValidation"
>
<input
aria-activedescendant=""
aria-describedby="input-validation-text"
aria-invalid={true}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand All @@ -950,6 +909,7 @@ exports[`TextField renders correctly > with validation 1`] = `
>
<span
className="subTextContainerStatus"
id="input-validation-text"
>
error
</span>
Expand All @@ -971,10 +931,8 @@ exports[`TextField renders correctly > with value 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down Expand Up @@ -1012,10 +970,8 @@ exports[`TextField renders correctly > with wrapperClassName 1`] = `
className="inputWrapper wrapperSizeSmall"
>
<input
aria-activedescendant=""
aria-invalid={false}
aria-label=""
aria-owns=""
aria-required={false}
autoComplete="off"
className="input"
Expand Down
6 changes: 4 additions & 2 deletions packages/docs/src/pages/components/TextField/TextField.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ import { TextField } from "@vibe/core";
clear to all users.
</>,
<>
When using <code>title</code> or validation text, you must also provide an <code>id</code>. This is crucial, as it
allows screen readers to correctly associate the input with its label and description.
When using <code>title</code> or validation text, you must also provide a unique <code>id</code>. This is crucial,
as the label is associated through it and the validation text is exposed to screen readers as{" "}
<code>aria-describedby="&#123;id&#125;-validation-text"</code>. Two fields sharing an <code>id</code> on the same
page produce duplicate description targets.
</>,
<>
For required fields, use the <code>required</code> prop to ensure proper screen reader announcements and native
Expand Down
Loading
Loading