From 013767045b565519965e4251fcaaae25405617ce Mon Sep 17 00:00:00 2001 From: Hunter Johnston Date: Thu, 14 Nov 2024 12:33:25 -0500 Subject: [PATCH] next: improve radio item selection --- .changeset/wild-dogs-attend.md | 5 ++ .../bits/radio-group/radio-group.svelte.ts | 10 +++ .../src/tests/radio-group/radio-group.test.ts | 61 ++++++++++++------- 3 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 .changeset/wild-dogs-attend.md diff --git a/.changeset/wild-dogs-attend.md b/.changeset/wild-dogs-attend.md new file mode 100644 index 000000000..5dce9929f --- /dev/null +++ b/.changeset/wild-dogs-attend.md @@ -0,0 +1,5 @@ +--- +"bits-ui": patch +--- + +fix: don't auto select first radio item unless there is already a selected value diff --git a/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts b/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts index 4978774b7..1d746b864 100644 --- a/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts +++ b/packages/bits-ui/src/lib/bits/radio-group/radio-group.svelte.ts @@ -8,6 +8,7 @@ import { useRovingFocus, } from "$lib/internal/use-roving-focus.svelte.js"; import { createContext } from "$lib/internal/create-context.js"; +import { kbd } from "$lib/internal/kbd.js"; const RADIO_GROUP_ROOT_ATTR = "data-radio-group-root"; const RADIO_GROUP_ITEM_ATTR = "data-radio-group-item"; @@ -32,6 +33,7 @@ class RadioGroupRootState { name: RadioGroupRootStateProps["name"]; value: RadioGroupRootStateProps["value"]; rovingFocusGroup: UseRovingFocusReturn; + hasValue = $derived.by(() => this.value.current !== ""); constructor(props: RadioGroupRootStateProps) { this.#id = props.id; @@ -42,6 +44,7 @@ class RadioGroupRootState { this.name = props.name; this.value = props.value; this.#ref = props.ref; + this.rovingFocusGroup = useRovingFocus({ rootNodeId: this.#id, candidateAttr: RADIO_GROUP_ITEM_ATTR, @@ -129,10 +132,17 @@ class RadioGroupItemState { }; #onfocus = () => { + if (!this.#root.hasValue) return; this.#root.setValue(this.#value.current); }; #onkeydown = (e: KeyboardEvent) => { + if (this.#isDisabled) return; + if (e.key === kbd.SPACE) { + e.preventDefault(); + this.#root.setValue(this.#value.current); + return; + } this.#root.rovingFocusGroup.handleKeydown(this.#ref.current, e, true); }; diff --git a/packages/tests/src/tests/radio-group/radio-group.test.ts b/packages/tests/src/tests/radio-group/radio-group.test.ts index 5cf9eadc5..977f05f79 100644 --- a/packages/tests/src/tests/radio-group/radio-group.test.ts +++ b/packages/tests/src/tests/radio-group/radio-group.test.ts @@ -91,19 +91,19 @@ describe("radio group", () => { const item2 = getByTestId(itemIds[2] as string); const item3 = getByTestId(itemIds[3] as string); item0.focus(); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); await user.keyboard(kbd.ARROW_DOWN); - await waitFor(() => expect(item1).toHaveFocus()); + expect(item1).toHaveFocus(); await user.keyboard(kbd.ARROW_DOWN); - await waitFor(() => expect(item2).toHaveFocus()); + expect(item2).toHaveFocus(); await user.keyboard(kbd.ARROW_DOWN); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); await user.keyboard(kbd.ARROW_UP); - await waitFor(() => expect(item2).toHaveFocus()); + expect(item2).toHaveFocus(); await user.keyboard(kbd.ARROW_UP); - await waitFor(() => expect(item1).toHaveFocus()); + expect(item1).toHaveFocus(); await user.keyboard(kbd.ARROW_UP); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); }); it("should navigate through the items using the keyboard (left and right)", async () => { @@ -114,19 +114,19 @@ describe("radio group", () => { const item2 = getByTestId(itemIds[2] as string); const item3 = getByTestId(itemIds[3] as string); item0.focus(); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - await waitFor(() => expect(item1).toHaveFocus()); + expect(item1).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - await waitFor(() => expect(item2).toHaveFocus()); + expect(item2).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); await user.keyboard(kbd.ARROW_LEFT); - await waitFor(() => expect(item2).toHaveFocus()); + expect(item2).toHaveFocus(); await user.keyboard(kbd.ARROW_LEFT); - await waitFor(() => expect(item1).toHaveFocus()); + expect(item1).toHaveFocus(); await user.keyboard(kbd.ARROW_LEFT); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); }); it("should respect the loop prop", async () => { @@ -137,14 +137,14 @@ describe("radio group", () => { const item0 = getByTestId(itemIds[0] as string); const item3 = getByTestId(itemIds[3] as string); item0.focus(); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); await user.keyboard(kbd.ARROW_UP); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); item3.focus(); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); await user.keyboard(kbd.ARROW_DOWN); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); }); it("should respect the value prop & binding", async () => { @@ -170,14 +170,14 @@ describe("radio group", () => { const item0 = getByTestId(itemIds[0] as string); const item3 = getByTestId(itemIds[3] as string); item0.focus(); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); await user.keyboard(kbd.ARROW_LEFT); - await waitFor(() => expect(item0).toHaveFocus()); + expect(item0).toHaveFocus(); item3.focus(); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); await user.keyboard(kbd.ARROW_RIGHT); - await waitFor(() => expect(item3).toHaveFocus()); + expect(item3).toHaveFocus(); }); it("should not render an input if the `name` prop isn't passed", async () => { @@ -220,4 +220,21 @@ describe("radio group", () => { expect(input).toHaveAttribute("disabled"); }); + + it("should not automatically select the first item focused when the radio group does not have a value", async () => { + const { getByTestId, user, input } = setup({ name: "radio-group" }); + + const aItem = getByTestId("a-item"); + aItem.focus(); + expect(input).toHaveValue(""); + await user.keyboard(kbd.ARROW_DOWN); + expect(input).toHaveValue(""); + const bItem = getByTestId("b-item"); + expect(bItem).toHaveFocus(); + await user.keyboard(kbd.SPACE); + expect(input).toHaveValue("b"); + await user.keyboard(kbd.ARROW_UP); + expect(aItem).toHaveFocus(); + expect(input).toHaveValue("a"); + }); });