From 59b8d558fa66e0577a8a98fb5aa4d61dd0042ee1 Mon Sep 17 00:00:00 2001 From: Grace Date: Mon, 3 Aug 2026 16:35:07 +0100 Subject: [PATCH 1/6] Remove &[data-focused] style --- packages/ui/src/Select.recipe.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index 5d10836..4d44f12 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -77,8 +77,6 @@ export const select = defineSlotRecipe({ focusShadow: "outline", borderColor: "focusBorder", }, - // 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" }, }, From 574b1f965ef042821d8fb3796da1d1db9690fb96 Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 4 Aug 2026 11:21:54 +0100 Subject: [PATCH 2/6] Treat combo selector like a text input so that when you mouse/keyboard focus on the selector, it changes border color without focus shadow. Also, remove borderColor hover effect. --- packages/ui/src/Select.recipe.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index d81373e..2afbaa3 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -72,12 +72,12 @@ export const select = defineSlotRecipe({ color: "inherit", h: "10", px: "4", - _hover: { borderColor: "gray.300" }, - "&[data-focus-visible]": { - focusShadow: "outline", + "&[data-invalid]": { borderColor: "danger.500" }, + "&[data-focus-visible], &:has(input[data-focused])": { borderColor: "focusBorder", + outline: "2px solid transparent", + outlineOffset: "2px", }, - "&[data-invalid]": { borderColor: "danger.500" }, "&[data-disabled]": { opacity: 0.4, cursor: "not-allowed" }, }, // Whatever shows the current value: Select's SelectValue, ComboBox's From 8cef43f9c8fe3d22232d44c2a511a8f209dafb2b Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 4 Aug 2026 11:27:09 +0100 Subject: [PATCH 3/6] Add comment --- packages/ui/src/Select.recipe.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index 2afbaa3..6c2e359 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -72,12 +72,18 @@ export const select = defineSlotRecipe({ color: "inherit", h: "10", px: "4", - "&[data-invalid]": { borderColor: "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 — we watch + // the input, which as a text field shows focus on any modality. Select's + // trigger contains no input, so the arm can't match it. "&[data-focus-visible], &:has(input[data-focused])": { borderColor: "focusBorder", outline: "2px solid transparent", outlineOffset: "2px", }, + "&[data-invalid]": { borderColor: "danger.500" }, "&[data-disabled]": { opacity: 0.4, cursor: "not-allowed" }, }, // Whatever shows the current value: Select's SelectValue, ComboBox's From 1caac096c4b719bca97fc1a6fa0484588f5bf72a Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 4 Aug 2026 11:33:29 +0100 Subject: [PATCH 4/6] Add boxShadow --- packages/ui/src/Select.recipe.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index 6c2e359..791a7b0 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -79,6 +79,7 @@ export const select = defineSlotRecipe({ // the input, which as a text field shows focus on any modality. Select's // trigger contains no input, so the arm can't match it. "&[data-focus-visible], &:has(input[data-focused])": { + boxShadow: "0 0 0 1px token(colors.focusBorder)", borderColor: "focusBorder", outline: "2px solid transparent", outlineOffset: "2px", From 593e86966ba48f124d882bf3a811d8cc3c9ed6d9 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Tue, 4 Aug 2026 13:15:42 +0100 Subject: [PATCH 5/6] Select: align the trigger's hover, invalid and focus states with Input Three state rules on the shared trigger slot, in the order the input recipe documents: hover, then invalid, then focus, so red beats a hover tint and the focus ring beats red. Focus keyed off `data-focused` never survived an open ComboBox list. react-aria dispatches a synthetic blur at the input when virtual focus moves to an option, so RAC clears the attribute while real focus has not moved. `menuTrigger="focus"` with a selection hits this on the first tab, which is why the icon story in #39 still had no ring; every ComboBox also lost it on arrowing into the list. Native `:focus` cannot be taken away this way. Invalid never matched anything. RAC marks the field root, and in a ComboBox the input, but never the trigger: its Button has no validity state and our ComboBox control is a plain div. The rule now comes down from the parent, with the border plus 1px ring the input recipe uses rather than the border alone. Scoped to a direct child so an app's own invalid form wrapper cannot paint every control inside it red. Hover returns. The input recipe carries it, and both NativeSelect and TextField's input render that recipe, so dropping it from the trigger left a Select the only control in a form that did not tint. The tests take these selectors from the recipe rather than restating them, so keying off an attribute RAC does not set fails there rather than in a browser. The Invalid story grows a form whose required Select and ComboBox go red on submit; its docstring records that red is the only signal a Select can give, which is #41. --- packages/ui/src/Select.recipe.ts | 29 ++++- packages/ui/stories/Select.stories.tsx | 47 +++++++- packages/ui/tests/Select.test.tsx | 150 ++++++++++++++++++++++++- 3 files changed, 216 insertions(+), 10 deletions(-) diff --git a/packages/ui/src/Select.recipe.ts b/packages/ui/src/Select.recipe.ts index 791a7b0..38a687f 100644 --- a/packages/ui/src/Select.recipe.ts +++ b/packages/ui/src/Select.recipe.ts @@ -72,19 +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-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 — we watch - // the input, which as a text field shows focus on any modality. Select's - // trigger contains no input, so the arm can't match it. - "&[data-focus-visible], &:has(input[data-focused])": { + // 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", }, - "&[data-invalid]": { borderColor: "danger.500" }, "&[data-disabled]": { opacity: 0.4, cursor: "not-allowed" }, }, // Whatever shows the current value: Select's SelectValue, ComboBox's 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(