diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index ce7bdc1..e6fabb9 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -72,14 +72,38 @@ export const select = defineSlotRecipe({ color: "inherit", h: "10", px: "4", + // As the input recipe, so a Select, a NativeSelect and a TextField in one + // form all tint together on hover. (react-aria's TextField has no hover + // effect, but matching the family beats matching their docs.) _hover: { borderColor: "gray.300" }, - "&[data-focus-visible]": { - focusShadow: "outline", + // `data-invalid` lands on the root — and, in a ComboBox, on the input — + // but never on the trigger: a RAC Button has no validity state, and our + // ComboBox control is a plain div. So it comes down from the parent. + // `> &` rather than a descendant selector, so an app's own invalid form + // wrapper cannot paint every control inside it red. + // + // Declared after hover and before focus so red beats a hover tint and + // the focus ring beats red, as in the input recipe. + "[data-invalid] > &": { + borderColor: "danger.500", + boxShadow: "0 0 0 1px token(colors.danger.500)", + }, + // Two focus cases. `data-focus-visible` is Select's button on keyboard + // focus only (RAC leaves it unset for mouse, matching the react-aria + // docs' Select). The `:has()` arm is ComboBox: its control is a plain + // div wrapping an input, so it gets no RAC attributes itself, and as a + // text field it should show focus on any modality. That arm watches + // native `:focus` rather than the input's `data-focused`, because + // react-aria dispatches a synthetic blur at the input whenever virtual + // focus moves to an option (aria-activedescendant) — which strips RAC's + // attribute for as long as the list has an active option, real focus + // never having left. Select's trigger holds no input, so it can't match. + "&[data-focus-visible], &:has(input:focus)": { + boxShadow: "0 0 0 1px token(colors.focusBorder)", borderColor: "focusBorder", + outline: "2px solid transparent", + outlineOffset: "2px", }, - // A ComboBox's control is an input, which is focused whenever it is open. - "&[data-focused]": { focusShadow: "outline", borderColor: "focusBorder" }, - "&[data-invalid]": { borderColor: "danger.500" }, "&[data-disabled]": { opacity: 0.4, cursor: "not-allowed" }, }, // Whatever shows the current value: Select's SelectValue, ComboBox's @@ -113,8 +137,11 @@ export const select = defineSlotRecipe({ background: "transparent", border: "none", cursor: "pointer", + // No focus styling, deliberately: react-aria keeps a ComboBox's toggle + // button out of the tab order (the input owns the keyboard), so a ring + // here would only ever be reachable programmatically, and would suggest + // the chevron is a tab stop. The whole control shows focus instead. outline: "none", - "&[data-focus-visible]": { focusShadow: "outline" }, }, content: { // Line the card up with the control, as a select should and as diff --git a/packages/ui/stories/Select.stories.tsx b/packages/ui/stories/Select.stories.tsx index b2ee389..3ff735a 100644 --- a/packages/ui/stories/Select.stories.tsx +++ b/packages/ui/stories/Select.stories.tsx @@ -6,7 +6,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { useState } from "react"; import { RiCloudLine, RiFireLine, RiSnowyLine } from "react-icons/ri"; -import { ComboBox, Icon, Select, SelectOption, Stack } from "../src"; +import { Button, ComboBox, Icon, Select, SelectOption, Stack } from "../src"; const meta = { title: "Forms/Select", @@ -191,16 +191,55 @@ export const Overridden: Story = { ), }; -/** Invalid state, as a form would set it. */ +/** + * Invalid state. `isInvalid` sets it directly; `isRequired` inside a form sets + * it on submit — the bottom pair here, which start clean, go red when you press + * Check with nothing chosen, and clear as soon as you choose something. + * + * Tab through them: the focus ring beats the red border while a control is + * focused, and hovering tints the border only while neither applies, both as a + * TextField or NativeSelect does. Note that red is the *only* signal a Select + * gives — unlike TextField it has no `errorMessage`, so anything explaining the + * error has to come from the app for now (#41). + */ export const Invalid: Story = { render: () => ( - {options} - + {options} + {/* Submitting empty marks both controls. They need a `name` to take part + in form validation at all. */} +
e.preventDefault()}> + + + {/* `isRequired`, not a `validate` rule: react-aria displays a + ComboBox's custom validation a step behind, so it goes red while + you are still typing and stays red after you have picked + something, until blur. */} + + {options} + + + +
), }; diff --git a/packages/ui/tests/Select.test.tsx b/packages/ui/tests/Select.test.tsx index c4b3ea2..2d3bf35 100644 --- a/packages/ui/tests/Select.test.tsx +++ b/packages/ui/tests/Select.test.tsx @@ -10,9 +10,10 @@ import { render, screen, } from "@testing-library/react"; -import { useState } from "react"; +import { ReactElement, useState } from "react"; import { afterEach, expect, it, vi } from "vitest"; import { ComboBox, Select, SelectOption } from "../src"; +import { select } from "../src/Select.recipe"; afterEach(cleanup); @@ -118,6 +119,153 @@ it("ComboBox shows an empty state and can drop the indicator", () => { expect(screen.getByText("Nothing found")).toBeDefined(); }); +/** + * The trigger slot's rules, keyed by the border colour each one sets, so these + * tests exercise the recipe's own selectors rather than copies of them. Panda's + * `&` is the element the rule lands on, which is what `matches` compares + * against. + */ +const triggerRules = () => + select.base?.trigger as Record; + +const ruleFor = (borderColor: string) => { + const rules = triggerRules(); + const selector = Object.keys(rules).find( + (k) => rules[k]?.borderColor === borderColor, + ); + expect(selector).toBeDefined(); + return selector!; +}; + +/** + * Fails if the focus rule goes back to keying off RAC's `data-focused`. That + * attribute is unusable while a ComboBox's list is open: react-aria dispatches + * a synthetic blur at the input when virtual focus moves to an option, so RAC + * drops it even though real focus never left. + */ +const isTriggerFocusStyled = (el: Element) => + el.matches(ruleFor("focusBorder").replaceAll("&", "*")); + +/** Fails if the invalid rule goes back to a `data-invalid` RAC never sets. */ +const isTriggerInvalidStyled = (el: Element) => + el.matches(ruleFor("danger.500").replaceAll("&", "*")); + +it("ComboBox keeps its focus styling while an option is active", () => { + render( + + {FRUIT.map((f) => ( + + {f} + + ))} + , + ); + const input = screen.getByRole("combobox") as HTMLInputElement; + act(() => input.focus()); + expect(input.getAttribute("aria-expanded")).toBe("true"); + expect(input.getAttribute("aria-activedescendant")).toBeTruthy(); + expect(input.getAttribute("data-focused")).toBeNull(); + + expect(isTriggerFocusStyled(input.parentElement!)).toBe(true); +}); + +it("ComboBox keeps its focus styling across opening, choosing and reopening", () => { + render( + + {FRUIT.map((f) => ( + + {f} + + ))} + , + ); + const input = screen.getByRole("combobox") as HTMLInputElement; + const trigger = input.parentElement!; + const toggle = screen.getByRole("button"); + act(() => input.focus()); + expect(isTriggerFocusStyled(trigger)).toBe(true); + + fireEvent.click(toggle); + expect(isTriggerFocusStyled(trigger)).toBe(true); + + fireEvent.click(screen.getByRole("option", { name: "Cherry" })); + expect(isTriggerFocusStyled(trigger)).toBe(true); + + // Reopening with a selection is the other way an option starts out active. + fireEvent.click(toggle); + expect(input.getAttribute("data-focused")).toBeNull(); + expect(isTriggerFocusStyled(trigger)).toBe(true); +}); + +it("Select's trigger takes focus styling from the keyboard only", () => { + renderSelect(); + const trigger = screen.getByRole("button"); + act(() => trigger.focus()); + // RAC sets data-focused for either modality, so the recipe keys off + // data-focus-visible; jsdom has no pointer, hence keyboard here. + expect(trigger.getAttribute("data-focus-visible")).toBe("true"); + expect(isTriggerFocusStyled(trigger)).toBe(true); + // The ComboBox arm must not reach a Select: no input inside the trigger. + expect(trigger.matches(":has(input)")).toBe(false); +}); + +const invalidCases: [string, ReactElement][] = [ + [ + "Select", + , + ], + [ + "ComboBox", + + Apple + , + ], +]; + +it.each(invalidCases)( + "an invalid %s paints its trigger from the root", + (_name, control) => { + const { container } = render(control); + // RAC marks the root, not the trigger, which is why the rule reaches down. + const root = container.querySelector('[class*="select__root"]')!; + const trigger = container.querySelector('[class*="select__trigger"]')!; + expect(root.getAttribute("data-invalid")).toBe("true"); + expect(trigger.getAttribute("data-invalid")).toBeNull(); + expect(isTriggerInvalidStyled(trigger)).toBe(true); + }, +); + +it("a valid control is not painted red", () => { + const { container } = render( + , + ); + const trigger = container.querySelector('[class*="select__trigger"]')!; + expect(isTriggerInvalidStyled(trigger)).toBe(false); +}); + +// jsdom applies no CSS, so the cascade can only be checked as declaration +// order: equal-specificity rules, so the last one wins. Chakra's behaviour, +// which the input recipe documents: red beats hover, the focus ring beats red. +it("orders the trigger's state rules hover, invalid, focus", () => { + const keys = Object.keys(triggerRules()); + const hover = keys.indexOf(ruleFor("gray.300")); + const invalid = keys.indexOf(ruleFor("danger.500")); + const focus = keys.indexOf(ruleFor("focusBorder")); + expect(hover).toBeLessThan(invalid); + expect(invalid).toBeLessThan(focus); +}); + it("drops the chevron when asked, rather than silently keeping it", () => { const { container } = render(