Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FormRadio error when not providing composite store #2313

Merged
merged 5 commits into from
Apr 25, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/large-goats-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@ariakit/react-core": patch
"@ariakit/react": patch
---

Fixed `FormRadio` error when not explicitly providing the composite store. ([#2313](https://github.com/ariakit/ariakit/pull/2313))
5 changes: 5 additions & 0 deletions .changeset/lemon-comics-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ariakit/core": patch
---

Added support for native radio buttons within forms, that work with roving tabindex, to all `focus` utilities. ([#2313](https://github.com/ariakit/ariakit/pull/2313))
26 changes: 26 additions & 0 deletions examples/form-radio/icon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
function getItem(textWidth = "w-10") {
return (
<div className="flex items-center gap-2">
<div className="h-2.5 w-2.5 rounded-full border-2 border-blue-600 bg-white dark:bg-black" />
<div className={`h-1 ${textWidth} bg-black/70 dark:bg-white/70`} />
</div>
);
}

export default function Icon() {
return (
<svg viewBox="0 0 128 128" width={128} height={128}>
<foreignObject width={128} height={128}>
<div className="flex h-full flex-col items-start justify-center gap-3 p-5">
<div className="h-1.5 w-12 bg-black/70 dark:bg-white/70" />
<div className="flex flex-col items-start gap-1">
{getItem("w-10")}
{getItem("w-8")}
{getItem("w-14")}
</div>
<div className="h-4 w-10 rounded-sm bg-blue-600" />
</div>
</foreignObject>
</svg>
);
}
45 changes: 45 additions & 0 deletions examples/form-radio/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { useId } from "react";
import * as Ariakit from "@ariakit/react";
import "./style.css";

export default function Example() {
const id = useId();
const form = Ariakit.useFormStore({ defaultValues: { color: "" } });

form.useValidate(() => {
if (!form.getValue(form.names.color)) {
form.setError(form.names.color, "Please select a color.");
}
});

form.useSubmit(async (state) => {
alert(JSON.stringify(state.values));
});

return (
<Ariakit.Form store={form} aria-labelledby={id} className="wrapper">
<h2 id={id} className="heading">
Preferences
</h2>
<Ariakit.FormRadioGroup className="radio-group">
<Ariakit.FormGroupLabel className="radio-group-label">
Favorite color
</Ariakit.FormGroupLabel>
<Ariakit.FormError name={form.names.color} className="error" />
<label className="radio-label">
<Ariakit.FormRadio name={form.names.color} value="red" />
Red
</label>
<label className="radio-label">
<Ariakit.FormRadio name={form.names.color} value="green" />
Green
</label>
<label className="radio-label">
<Ariakit.FormRadio name={form.names.color} value="blue" />
Blue
</label>
</Ariakit.FormRadioGroup>
<Ariakit.FormSubmit className="button">Submit</Ariakit.FormSubmit>
</Ariakit.Form>
);
}
7 changes: 7 additions & 0 deletions examples/form-radio/readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# FormRadio

<p data-description>
Using the <a href="/apis/form-radio-group"><code>FormRadioGroup</code></a> and <a href="/apis/form-radio"><code>FormRadio</code></a> components to create a <a href="/components/form">Form</a> with custom validation that requires a user to select an option from a list of radio buttons.
</p>

<a href="./index.tsx" data-playground>Example</a>
39 changes: 39 additions & 0 deletions examples/form-radio/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
@import url("../form/style.css");

.radio-group {
@apply
flex
flex-col
items-start
gap-2
;
}

.radio-group-label {
@apply
text-sm
font-semibold
text-black/70
dark:text-white/70
;
}

.radio-label {
@apply
flex
items-center
gap-2
;
}

.button {
@apply
flex-none
;
}

.error {
@apply
w-full
;
}
69 changes: 69 additions & 0 deletions examples/form-radio/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { click, getByRole, press, queryByText } from "@ariakit/test";

const getSubmit = () => getByRole("button", { name: "Submit" });
const getRadio = (name: string) => getByRole("radio", { name });
const getError = () => queryByText("Please select a color.");

const spyOnAlert = () => vi.spyOn(window, "alert").mockImplementation(() => {});

let alert = spyOnAlert();

beforeEach(() => {
alert = spyOnAlert();
return () => alert.mockClear();
});

test("focus on the first radio button by tabbing", async () => {
expect(getRadio("Red")).not.toHaveFocus();
expect(getRadio("Green")).not.toHaveFocus();
expect(getRadio("Blue")).not.toHaveFocus();
await press.Tab();
expect(getRadio("Red")).toHaveFocus();
});

test("show error on blur", async () => {
await press.Tab();
expect(getError()).not.toBeInTheDocument();
await press.Tab();
expect(getSubmit()).toHaveFocus();
expect(getError()).toBeInTheDocument();
});

test("show error on submit", async () => {
expect(getError()).not.toBeInTheDocument();
await click(getSubmit());
expect(getError()).toBeInTheDocument();
});

test("focus on radio with error on submit", async () => {
await click(getSubmit());
expect(getRadio("Red")).toHaveFocus();
});

test("fix error on change", async () => {
await press.Tab();
expect(getRadio("Red")).toHaveFocus();
await press.Tab();
expect(getSubmit()).toHaveFocus();
await press.ShiftTab();
expect(getRadio("Blue")).toHaveFocus();
expect(getError()).toBeInTheDocument();
await press.Space();
expect(getError()).not.toBeInTheDocument();
});

test("submit form", async () => {
await click(getRadio("Green"));
await click(getSubmit());
expect(alert).toHaveBeenCalledWith(JSON.stringify({ color: "green" }));
});

test("reset form on submit", async () => {
await press.Tab();
await press.Space();
await press.Tab();
await press.Enter();
expect(getRadio("Red")).not.toBeChecked();
expect(getRadio("Green")).not.toBeChecked();
expect(getRadio("Blue")).not.toBeChecked();
});
4 changes: 2 additions & 2 deletions examples/form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import "./style.css";
export default function Example() {
const form = Ariakit.useFormStore({ defaultValues: { name: "", email: "" } });

form.useSubmit(async () => {
alert(JSON.stringify(form.getState().values));
form.useSubmit(async (state) => {
alert(JSON.stringify(state.values));
});

return (
Expand Down
11 changes: 3 additions & 8 deletions examples/form/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,9 @@
;
}

.error:empty {
@apply
absolute
;
}

.error:not(:empty) {
.error {
@apply
empty:hidden
w-fit
rounded-md
py-2
Expand Down Expand Up @@ -94,7 +89,7 @@
;
}

.button.reset {
.reset {
@apply
font-semibold
text-black/70
Expand Down
2 changes: 1 addition & 1 deletion examples/radio/icon.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
function getItem(filled = false) {
return (
<div className="flex items-center gap-2">
<div className="h-4 w-4 rounded-full border-2 border-blue-600 bg-white p-0.5 dark:border-blue-600 dark:bg-black">
<div className="h-4 w-4 rounded-full border-2 border-blue-600 bg-white p-0.5 dark:bg-black">
{filled && (
<div className="h-full w-full rounded-full bg-blue-600 dark:bg-blue-500" />
)}
Expand Down
30 changes: 28 additions & 2 deletions packages/ariakit-core/src/utils/focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,34 @@ export function isFocusable(element: Element): element is HTMLElement {
* isTabbable(document.querySelector("input[hidden]")); // false
* isTabbable(document.querySelector("input:disabled")); // false
*/
export function isTabbable(element: Element): element is HTMLElement {
return isFocusable(element) && !hasNegativeTabIndex(element);
export function isTabbable(
element: Element | HTMLElement | HTMLInputElement
): element is HTMLElement {
if (!isFocusable(element)) return false;
if (hasNegativeTabIndex(element)) return false;
// If the element is a radio button in a form, we must take roving tabindex
// into account.
if (!("form" in element)) return true;
if (!element.form) return true;
if (element.checked) return true;
if (element.type !== "radio") return true;
// If the radio button is not part of a radio group, it's tabbable.
const radioGroup = element.form.elements.namedItem(element.name);
if (!radioGroup) return true;
if (!("length" in radioGroup)) return true;
// If we are in a radio group, we must check if the active element is part of
// the same group, which would make all the other radio buttons in the group
// non-tabbable.
const activeElement = getActiveElement(element) as
| HTMLElement
| HTMLInputElement
| null;
if (!activeElement) return true;
if (activeElement === element) return true;
if (!("form" in activeElement)) return true;
if (activeElement.form !== element.form) return true;
if (activeElement.name !== element.name) return true;
return false;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { RefCallback } from "react";
import { useCallback, useContext, useRef } from "react";
import type { CollectionStoreItem } from "@ariakit/core/collection/collection-store";
import { identity, invariant } from "@ariakit/core/utils/misc";
import { identity } from "@ariakit/core/utils/misc";
import { useForkRef, useId } from "../utils/hooks.js";
import { createComponent, createElement, createHook } from "../utils/system.js";
import type { As, Options, Props } from "../utils/types.js";
Expand All @@ -26,12 +26,6 @@ export const useCollectionItem = createHook<CollectionItemOptions>(
const context = useContext(CollectionContext);
store = store || context;

invariant(
store,
process.env.NODE_ENV !== "production" &&
"CollectionItem must be wrapped in a Collection component"
);

const id = useId(props.id);
const unrenderItem = useRef<() => void>();

Expand Down
22 changes: 9 additions & 13 deletions packages/ariakit-react-core/src/composite/composite-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
isTextField,
} from "@ariakit/core/utils/dom";
import { isPortalEvent, isSelfTarget } from "@ariakit/core/utils/events";
import { invariant } from "@ariakit/core/utils/misc";
import type { BooleanOrCallback } from "@ariakit/core/utils/types";
import type { CollectionItemOptions } from "../collection/collection-item.js";
import { useCollectionItem } from "../collection/collection-item.js";
Expand All @@ -25,6 +24,7 @@ import {
useSafeLayoutEffect,
useWrapElement,
} from "../utils/hooks.js";
import { useStoreState } from "../utils/store.js";
import {
createElement,
createHook,
Expand Down Expand Up @@ -170,16 +170,10 @@ export const useCompositeItem = createHook<CompositeItemOptions>(
const context = useContext(CompositeContext);
store = store || context;

invariant(
store,
process.env.NODE_ENV !== "production" &&
"CompositeItem must be wrapped in a Composite component"
);

const id = useId(props.id);
const ref = useRef<HTMLButtonElement>(null);
const row = useContext(CompositeRowContext);
const rowId = store.useState((state) => {
const rowId = useStoreState(store, (state) => {
if (rowIdProp) return rowIdProp;
if (!row?.baseElement) return;
if (row.baseElement !== state.baseElement) return;
Expand Down Expand Up @@ -316,7 +310,8 @@ export const useCompositeItem = createHook<CompositeItemOptions>(
}
});

const baseElement = store.useState(
const baseElement = useStoreState(
store,
(state) => state.baseElement || undefined
);

Expand All @@ -335,9 +330,9 @@ export const useCompositeItem = createHook<CompositeItemOptions>(
[providerValue]
);

const isActiveItem = store.useState((state) => state.activeId === id);
const isActiveItem = useStoreState(store, (state) => state.activeId === id);
const role = useRole(ref, props);
const virtualFocus = store.useState("virtualFocus");
const virtualFocus = useStoreState(store, "virtualFocus");
let ariaSelected: boolean | undefined;

if (isActiveItem) {
Expand All @@ -354,7 +349,8 @@ export const useCompositeItem = createHook<CompositeItemOptions>(
}
}

const shouldTabIndex = store.useState(
const shouldTabIndex = useStoreState(
store,
(state) => !state.virtualFocus && isActiveItem
);

Expand All @@ -364,7 +360,7 @@ export const useCompositeItem = createHook<CompositeItemOptions>(
"data-active-item": isActiveItem ? "" : undefined,
...props,
ref: useForkRef(ref, props.ref),
tabIndex: shouldTabIndex ? props.tabIndex : -1,
tabIndex: shouldTabIndex !== false ? props.tabIndex : -1,
onFocus,
onBlurCapture,
onKeyDown,
Expand Down