From 7d6a1aa12ea62bca067ab1ee7478abf5aab828f8 Mon Sep 17 00:00:00 2001 From: chirokas <157580465+chirokas@users.noreply.github.com> Date: Mon, 4 Nov 2024 15:17:53 +0000 Subject: [PATCH 1/3] fix(select): prevent default browser error UI from appearing --- .changeset/quick-buses-kick.md | 5 +++++ packages/components/select/src/hidden-select.tsx | 7 +------ 2 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 .changeset/quick-buses-kick.md diff --git a/.changeset/quick-buses-kick.md b/.changeset/quick-buses-kick.md new file mode 100644 index 0000000000..b0eff9da88 --- /dev/null +++ b/.changeset/quick-buses-kick.md @@ -0,0 +1,5 @@ +--- +"@nextui-org/select": patch +--- + +Prevent default browser error UI from appearing. diff --git a/packages/components/select/src/hidden-select.tsx b/packages/components/select/src/hidden-select.tsx index d7c74ef43a..a45197fbff 100644 --- a/packages/components/select/src/hidden-select.tsx +++ b/packages/components/select/src/hidden-select.tsx @@ -92,15 +92,9 @@ export function useHiddenSelect( inputProps: { type: "text", tabIndex: modality == null || state.isFocused || state.isOpen ? -1 : 0, - autoComplete, - value: [...state.selectedKeys].join(",") ?? "", - required: isRequired, style: {fontSize: 16}, onFocus: () => triggerRef.current?.focus(), disabled: isDisabled, - // The onChange is handled by the `select` element. This avoids the `form` with input `value` - // and no `onChange` warning. - onChange: () => {}, }, selectProps: { name, @@ -108,6 +102,7 @@ export function useHiddenSelect( autoComplete, // TODO: Address validation for cases where an option is selected and then deselected. // required: validationBehavior === "native" && isRequired, + required: isRequired, disabled: isDisabled, size: state.collection.size, value: From b477f746030159f6f5b01d073942534af651d715 Mon Sep 17 00:00:00 2001 From: chirokas Date: Tue, 5 Nov 2024 23:33:05 +0800 Subject: [PATCH 2/3] fix(select): improve form submit --- .../select/__tests__/select.test.tsx | 30 +++++++++++++++++++ .../components/select/src/hidden-select.tsx | 1 - 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/components/select/__tests__/select.test.tsx b/packages/components/select/__tests__/select.test.tsx index 6776344eeb..735a5b38c1 100644 --- a/packages/components/select/__tests__/select.test.tsx +++ b/packages/components/select/__tests__/select.test.tsx @@ -882,3 +882,33 @@ describe("Select with React Hook Form", () => { expect(onSubmit).toHaveBeenCalledTimes(1); }); }); + +describe("validationBehavior=native", () => { + it("should not submit form when required field is empty", async () => { + const onSubmit = jest.fn(); + + const {getByTestId} = render( +
+ + +
, + ); + + const user = userEvent.setup(); + + await user.click(getByTestId("button")); + + expect(onSubmit).toHaveBeenCalledTimes(0); + }); +}); diff --git a/packages/components/select/src/hidden-select.tsx b/packages/components/select/src/hidden-select.tsx index a45197fbff..9d24dd3949 100644 --- a/packages/components/select/src/hidden-select.tsx +++ b/packages/components/select/src/hidden-select.tsx @@ -104,7 +104,6 @@ export function useHiddenSelect( // required: validationBehavior === "native" && isRequired, required: isRequired, disabled: isDisabled, - size: state.collection.size, value: selectionMode === "multiple" ? [...state.selectedKeys].map((k) => String(k)) From 5bf7332a4a8326bd61443f3c0cee69b349d5c7bd Mon Sep 17 00:00:00 2001 From: chirokas Date: Wed, 6 Nov 2024 16:21:13 +0800 Subject: [PATCH 3/3] chore: cleanup --- .changeset/quick-buses-kick.md | 3 +- .../select/__tests__/select.test.tsx | 31 +++++++++++++++++-- .../src/use-multiselect-state.ts | 3 +- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/.changeset/quick-buses-kick.md b/.changeset/quick-buses-kick.md index b0eff9da88..0ed0c701c0 100644 --- a/.changeset/quick-buses-kick.md +++ b/.changeset/quick-buses-kick.md @@ -1,5 +1,6 @@ --- "@nextui-org/select": patch +"@nextui-org/use-aria-multiselect": patch --- -Prevent default browser error UI from appearing. +Prevent default browser error UI from appearing (#3913). diff --git a/packages/components/select/__tests__/select.test.tsx b/packages/components/select/__tests__/select.test.tsx index 735a5b38c1..ecc974f825 100644 --- a/packages/components/select/__tests__/select.test.tsx +++ b/packages/components/select/__tests__/select.test.tsx @@ -884,7 +884,13 @@ describe("Select with React Hook Form", () => { }); describe("validationBehavior=native", () => { - it("should not submit form when required field is empty", async () => { + let user: UserEvent; + + beforeEach(() => { + user = userEvent.setup(); + }); + + it("supports server validation", async () => { const onSubmit = jest.fn(); const {getByTestId} = render( @@ -893,6 +899,7 @@ describe("validationBehavior=native", () => { isRequired data-testid="select" label="Test" + name="select" // validationBehavior="native" > One @@ -905,10 +912,28 @@ describe("validationBehavior=native", () => { , ); - const user = userEvent.setup(); + const button = getByTestId("button"); + const select = getByTestId("select"); + const input = document.querySelector("[name=select]"); + + expect(input).toHaveAttribute("required"); + expect(select).not.toHaveAttribute("aria-describedby"); - await user.click(getByTestId("button")); + await user.click(button); + expect(select).toHaveAttribute("aria-describedby"); + expect(input.validity.valid).toBe(false); expect(onSubmit).toHaveBeenCalledTimes(0); + + expect(document.activeElement).toBe(select); + + await user.keyboard("[ArrowRight]"); + + expect(select).not.toHaveAttribute("aria-describedby"); + expect(input.validity.valid).toBe(true); + + await user.click(button); + + expect(onSubmit).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts b/packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts index 8e2d5535c6..a4c371871e 100644 --- a/packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts +++ b/packages/hooks/use-aria-multiselect/src/use-multiselect-state.ts @@ -71,6 +71,8 @@ export function useMultiSelectState(props: MultiSelectProps): M if (props.selectionMode === "single") { triggerState.close(); } + + validationState.commitValidation(); }, }); @@ -101,7 +103,6 @@ export function useMultiSelectState(props: MultiSelectProps): M if (listState.collection.size !== 0) { setFocusStrategy(focusStrategy); triggerState.toggle(); - validationState.commitValidation(); } }, isFocused,