From b530806867a5a172888877864c53832ad4991395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Tue, 17 Sep 2024 22:31:03 +0900 Subject: [PATCH 1/8] fix(input): ensure clear button is not focusable when disabled --- packages/components/input/src/use-input.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/input/src/use-input.ts b/packages/components/input/src/use-input.ts index 256c75b8ec..db54515c45 100644 --- a/packages/components/input/src/use-input.ts +++ b/packages/components/input/src/use-input.ts @@ -511,7 +511,7 @@ export function useInput Date: Tue, 17 Sep 2024 22:32:37 +0900 Subject: [PATCH 2/8] test(input): add test to ensure clear button is not focusable when disabled --- packages/components/input/__tests__/input.test.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/components/input/__tests__/input.test.tsx b/packages/components/input/__tests__/input.test.tsx index 23859a06d5..224bf917e9 100644 --- a/packages/components/input/__tests__/input.test.tsx +++ b/packages/components/input/__tests__/input.test.tsx @@ -37,6 +37,14 @@ describe("Input", () => { expect(container.querySelector("input")).toHaveAttribute("disabled"); }); + it("should set tabIndex to -1 when isDisabled is true", () => { + const {getByRole} = render(); + + const clearButton = getByRole("button"); + + expect(clearButton).toHaveAttribute("tabIndex", "-1"); + }); + it("should have required attribute when isRequired with native validationBehavior", () => { const {container} = render(); From 2823aead183182c1eeb9b636903e1dd782dd7c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Tue, 17 Sep 2024 22:38:40 +0900 Subject: [PATCH 3/8] chore: add changeset for clear button focus fix when input is disabled --- .changeset/two-waves-own.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/two-waves-own.md diff --git a/.changeset/two-waves-own.md b/.changeset/two-waves-own.md new file mode 100644 index 0000000000..8c534e7b7f --- /dev/null +++ b/.changeset/two-waves-own.md @@ -0,0 +1,5 @@ +--- +"@nextui-org/input": patch +--- + +clear button should not receive focus when input is disabled. From 4716c45220342eacfff74bb8eb52998749910add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Fri, 20 Sep 2024 00:37:09 +0900 Subject: [PATCH 4/8] fix(input): update clear button to use button element --- packages/components/input/src/input.tsx | 2 +- packages/components/input/src/use-input.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/input/src/input.tsx b/packages/components/input/src/input.tsx index 9414472494..10c5ecf10e 100644 --- a/packages/components/input/src/input.tsx +++ b/packages/components/input/src/input.tsx @@ -36,7 +36,7 @@ const Input = forwardRef<"input", InputProps>((props, ref) => { const end = useMemo(() => { if (isClearable) { - return {endContent || }; + return ; } return endContent; diff --git a/packages/components/input/src/use-input.ts b/packages/components/input/src/use-input.ts index db54515c45..3cc78e0d8f 100644 --- a/packages/components/input/src/use-input.ts +++ b/packages/components/input/src/use-input.ts @@ -510,8 +510,8 @@ export function useInput { return { ...props, - role: "button", - tabIndex: originalProps?.isDisabled ? -1 : 0, + type: "button", + disabled: originalProps.isDisabled, "aria-label": "clear input", "data-slot": "clear-button", "data-focus-visible": dataAttr(isClearButtonFocusVisible), From 081c0e416dc8f0bd507b6933a46bc3addf1559db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Fri, 20 Sep 2024 00:39:51 +0900 Subject: [PATCH 5/8] test(input): add focus test when disabled and update tests for clear button using button element --- .../components/input/__tests__/input.test.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/components/input/__tests__/input.test.tsx b/packages/components/input/__tests__/input.test.tsx index 224bf917e9..799295aa22 100644 --- a/packages/components/input/__tests__/input.test.tsx +++ b/packages/components/input/__tests__/input.test.tsx @@ -37,12 +37,12 @@ describe("Input", () => { expect(container.querySelector("input")).toHaveAttribute("disabled"); }); - it("should set tabIndex to -1 when isDisabled is true", () => { - const {getByRole} = render(); + it("should disable the clear button when isDisabled", () => { + const {container} = render(); - const clearButton = getByRole("button"); + const clearButton = container.querySelector("button"); - expect(clearButton).toHaveAttribute("tabIndex", "-1"); + expect(clearButton).toBeDisabled(); }); it("should have required attribute when isRequired with native validationBehavior", () => { @@ -139,7 +139,7 @@ describe("Input", () => { const ref = React.createRef(); - const {getByRole} = render( + const {container} = render( { />, ); - const clearButton = getByRole("button"); + const clearButton = container.querySelector("button")!; expect(clearButton).not.toBeNull(); @@ -194,7 +194,7 @@ describe("Input", () => { const ref = React.createRef(); - const {getByRole} = render( + const {container} = render( { />, ); - const clearButton = getByRole("button"); + const clearButton = container.querySelector("button")!; expect(clearButton).not.toBeNull(); @@ -264,7 +264,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", () => { From b3bd46ab9434449c5db01f1f9daf0d1dd893ed9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Sat, 21 Sep 2024 16:07:38 +0900 Subject: [PATCH 6/8] test(input): replace querySelector with getByRole for clear button --- packages/components/input/__tests__/input.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/components/input/__tests__/input.test.tsx b/packages/components/input/__tests__/input.test.tsx index 799295aa22..721fdb2883 100644 --- a/packages/components/input/__tests__/input.test.tsx +++ b/packages/components/input/__tests__/input.test.tsx @@ -38,9 +38,9 @@ describe("Input", () => { }); it("should disable the clear button when isDisabled", () => { - const {container} = render(); + const {getByRole} = render(); - const clearButton = container.querySelector("button"); + const clearButton = getByRole("button"); expect(clearButton).toBeDisabled(); }); @@ -139,7 +139,7 @@ describe("Input", () => { const ref = React.createRef(); - const {container} = render( + const {getByRole} = render( { />, ); - const clearButton = container.querySelector("button")!; + const clearButton = getByRole("button")!; expect(clearButton).not.toBeNull(); @@ -194,7 +194,7 @@ describe("Input", () => { const ref = React.createRef(); - const {container} = render( + const {getByRole} = render( { />, ); - const clearButton = container.querySelector("button")!; + const clearButton = getByRole("button")!; expect(clearButton).not.toBeNull(); From 84fd6b4560ef3479e89215897daa6a6035407b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EB=8F=84=EB=A5=9C?= <88328436+ryxxn@users.noreply.github.com> Date: Wed, 25 Sep 2024 13:04:27 +0900 Subject: [PATCH 7/8] fix(input): set tabIndex to -1 for clear button --- packages/components/input/src/use-input.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/input/src/use-input.ts b/packages/components/input/src/use-input.ts index 3cc78e0d8f..26e0e15eff 100644 --- a/packages/components/input/src/use-input.ts +++ b/packages/components/input/src/use-input.ts @@ -511,6 +511,7 @@ export function useInput Date: Thu, 26 Sep 2024 01:13:27 +0900 Subject: [PATCH 8/8] test(input): ensure clear button is not focusable --- packages/components/input/__tests__/input.test.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/components/input/__tests__/input.test.tsx b/packages/components/input/__tests__/input.test.tsx index 721fdb2883..5f967d25fb 100644 --- a/packages/components/input/__tests__/input.test.tsx +++ b/packages/components/input/__tests__/input.test.tsx @@ -45,6 +45,14 @@ describe("Input", () => { expect(clearButton).toBeDisabled(); }); + it("should not allow clear button to be focusable", () => { + const {getByRole} = render(); + + const clearButton = getByRole("button"); + + expect(clearButton).toHaveAttribute("tabIndex", "-1"); + }); + it("should have required attribute when isRequired with native validationBehavior", () => { const {container} = render();