Skip to content

Commit

Permalink
fix(input): ensure clear button is not focusable when disabled (#3774)
Browse files Browse the repository at this point in the history
* fix(input): ensure clear button is not focusable when disabled

* test(input): add test to ensure clear button is not focusable when disabled

* chore: add changeset for clear button focus fix when input is disabled

* fix(input): update clear button to use button element

* test(input): add focus test when disabled

and update tests for clear button using button element

* test(input): replace querySelector with getByRole for clear button

* fix(input): set tabIndex to -1 for clear button

* test(input): ensure clear button is not focusable
  • Loading branch information
ryxxn authored Sep 26, 2024
1 parent 3f8b63e commit 606f757
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/two-waves-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@nextui-org/input": patch
---

clear button should not receive focus when input is disabled.
22 changes: 19 additions & 3 deletions packages/components/input/__tests__/input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ describe("Input", () => {
expect(container.querySelector("input")).toHaveAttribute("disabled");
});

it("should disable the clear button when isDisabled", () => {
const {getByRole} = render(<Input isClearable isDisabled label="test input" />);

const clearButton = getByRole("button");

expect(clearButton).toBeDisabled();
});

it("should not allow clear button to be focusable", () => {
const {getByRole} = render(<Input isClearable label="test input" />);

const clearButton = getByRole("button");

expect(clearButton).toHaveAttribute("tabIndex", "-1");
});

it("should have required attribute when isRequired with native validationBehavior", () => {
const {container} = render(<Input isRequired label="test input" validationBehavior="native" />);

Expand Down Expand Up @@ -141,7 +157,7 @@ describe("Input", () => {
/>,
);

const clearButton = getByRole("button");
const clearButton = getByRole("button")!;

expect(clearButton).not.toBeNull();

Expand Down Expand Up @@ -197,7 +213,7 @@ describe("Input", () => {
/>,
);

const clearButton = getByRole("button");
const clearButton = getByRole("button")!;

expect(clearButton).not.toBeNull();

Expand Down Expand Up @@ -256,7 +272,7 @@ describe("Input with React Hook Form", () => {
input1 = document.querySelector("input[name=withDefaultValue]")!;
input2 = document.querySelector("input[name=withoutDefaultValue]")!;
input3 = document.querySelector("input[name=requiredField]")!;
submitButton = document.querySelector("button")!;
submitButton = document.querySelector('button[type="submit"]')!;
});

it("should work with defaultValues", () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/input/src/input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const Input = forwardRef<"input", InputProps>((props, ref) => {

const end = useMemo(() => {
if (isClearable) {
return <span {...getClearButtonProps()}>{endContent || <CloseFilledIcon />}</span>;
return <button {...getClearButtonProps()}>{endContent || <CloseFilledIcon />}</button>;
}

return endContent;
Expand Down
5 changes: 3 additions & 2 deletions packages/components/input/src/use-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,9 @@ export function useInput<T extends HTMLInputElement | HTMLTextAreaElement = HTML
(props = {}) => {
return {
...props,
role: "button",
tabIndex: 0,
type: "button",
tabIndex: -1,
disabled: originalProps.isDisabled,
"aria-label": "clear input",
"data-slot": "clear-button",
"data-focus-visible": dataAttr(isClearButtonFocusVisible),
Expand Down

0 comments on commit 606f757

Please sign in to comment.