Update selector focus styling and behaviour

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

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: () => (
     <Stack gap={5} css={{ maxWidth: "16rem" }}>
-      <Select label="Fruit" placeholder="Select…" isInvalid>
+      <Select label="Fruit (invalid)" placeholder="Select…" isInvalid>
         {options}
       </Select>
-      <ComboBox label="Fruit" placeholder="Start typing…" isInvalid>
+      <ComboBox label="Fruit (invalid)" placeholder="Start typing…" isInvalid>
         {options}
       </ComboBox>
+      {/* Submitting empty marks both controls. They need a `name` to take part
+          in form validation at all. */}
+      <form onSubmit={(e) => e.preventDefault()}>
+        <Stack gap={5}>
+          <Select
+            label="Fruit (required)"
+            placeholder="Select…"
+            name="a"
+            isRequired
+          >
+            {options}
+          </Select>
+          {/* `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. */}
+          <ComboBox
+            label="Fruit (required)"
+            placeholder="Start typing…"
+            name="b"
+            isRequired
+          >
+            {options}
+          </ComboBox>
+          <Button type="submit" variant="secondary">
+            Check
+          </Button>
+        </Stack>
+      </form>
     </Stack>
   ),
 };

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<string, { borderColor?: string } | undefined>;
+
+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(
+    <ComboBox
+      aria-label="Fruit"
+      // The shape of the stories that regressed: opening on focus with
+      // something already chosen means an option is active from the first tab.
+      menuTrigger="focus"
+      defaultSelectedKey="Banana"
+      defaultInputValue="Banana"
+    >
+      {FRUIT.map((f) => (
+        <SelectOption key={f} id={f}>
+          {f}
+        </SelectOption>
+      ))}
+    </ComboBox>,
+  );
+  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(
+    <ComboBox aria-label="Fruit">
+      {FRUIT.map((f) => (
+        <SelectOption key={f} id={f}>
+          {f}
+        </SelectOption>
+      ))}
+    </ComboBox>,
+  );
+  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",
+    <Select aria-label="Fruit" isInvalid>
+      <SelectOption id="a">Apple</SelectOption>
+    </Select>,
+  ],
+  [
+    "ComboBox",
+    <ComboBox aria-label="Fruit" isInvalid>
+      <SelectOption id="a">Apple</SelectOption>
+    </ComboBox>,
+  ],
+];
+
+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(
+    <Select aria-label="Fruit">
+      <SelectOption id="a">Apple</SelectOption>
+    </Select>,
+  );
+  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(
     <Select aria-label="Fruit" indicator={null}>