fix(ui-kit): move focus ring to focus-visible on Select/Dialog/Sheet/NavigationMenu (#8335)

SelectTrigger, DialogPrimitive.Close, SheetPrimitive.Close, and
NavigationMenuTrigger applied their focus ring/highlight on plain focus:,
which also fires on mouse-click focus and leaves a lingering ring/highlight
after every click — unlike button/input/switch/checkbox/toggle/slider and
every other interactive primitive in the kit, which ring on focus-visible:.
Moves only the visible ring/highlight classes to focus-visible: (keeping
focus:outline-none, which correctly clears the native outline on any focus),
with a regression test asserting the corrected class lists.

Closes #8304

Co-authored-by: RealDiligent <nft.gold.eth@gmail.com>

Author: RealDiligent

packages/loopover-ui-kit/src/components/dialog.tsx

@@ -44,7 +44,7 @@ const DialogContent = React.forwardRef<
       {...props}
     >
       {children}
-      <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background cursor-pointer transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground">
+      <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background cursor-pointer transition-opacity hover:opacity-100 focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground">
         <X className="h-4 w-4" />
         <span className="sr-only">Close</span>
       </DialogPrimitive.Close>

packages/loopover-ui-kit/src/components/focus-visible-convention.test.tsx

@@ -0,0 +1,39 @@
+import { render } from "@testing-library/react";
+import { describe, expect, it } from "vitest";
+
+import { navigationMenuTriggerStyle } from "./navigation-menu";
+import { Select, SelectTrigger, SelectValue } from "./select";
+
+// Regression for #8304: SelectTrigger, the Dialog/Sheet close buttons, and NavigationMenuTrigger must
+// apply their focus ring/highlight via `focus-visible:` (keyboard/programmatic focus only), matching
+// every other interactive primitive in @loopover/ui-kit — never on plain `focus:`, which also fires on
+// mouse-click focus and leaves a lingering ring/highlight. `focus:outline-none` is intentionally kept
+// (clearing the native outline on any focus is correct and shared by every primitive).
+describe("focus-visible convention (#8304)", () => {
+  it("navigationMenuTriggerStyle highlights on focus-visible, never a bare focus:bg-accent", () => {
+    const classes = navigationMenuTriggerStyle();
+    expect(classes).toContain("focus-visible:bg-accent");
+    expect(classes).toContain("focus-visible:text-accent-foreground");
+    // No bare focus:bg-accent / focus:text-accent-foreground (the data-[state=open]:focus:bg-accent
+    // compound is a separate, intentional open-state rule and is allowed).
+    expect(classes).not.toMatch(/(?<!:)\bfocus:bg-accent\b/);
+    expect(classes).not.toMatch(/(?<!:)\bfocus:text-accent-foreground\b/);
+    // The native-outline clear stays on plain focus:.
+    expect(classes).toContain("focus:outline-none");
+  });
+
+  it("SelectTrigger rings on focus-visible, never a bare focus:ring", () => {
+    const { getByRole } = render(
+      <Select>
+        <SelectTrigger aria-label="pick">
+          <SelectValue placeholder="pick" />
+        </SelectTrigger>
+      </Select>,
+    );
+    const trigger = getByRole("combobox");
+    expect(trigger.className).toContain("focus-visible:ring-1");
+    expect(trigger.className).toContain("focus-visible:ring-ring");
+    expect(trigger.className).not.toMatch(/(?<!-)\bfocus:ring/);
+    expect(trigger.className).toContain("focus:outline-none");
+  });
+});

packages/loopover-ui-kit/src/components/navigation-menu.tsx

@@ -41,7 +41,7 @@ NavigationMenuList.displayName = NavigationMenuPrimitive.List.displayName;
 const NavigationMenuItem = NavigationMenuPrimitive.Item;
 
 const navigationMenuTriggerStyle = cva(
-  "group inline-flex h-9 w-max items-center justify-center rounded-md bg-background px-4 py-2 text-sm font-medium cursor-pointer transition-colors hover:bg-accent hover:text-accent-foreground focus:bg-accent focus:text-accent-foreground focus:outline-none disabled:pointer-events-none disabled:opacity-50 disabled:cursor-not-allowed data-[state=open]:text-accent-foreground data-[state=open]:bg-accent/50 data-[state=open]:hover:bg-accent data-[state=open]:focus:bg-accent",
+  "group inline-flex h-9 w-max items-center justify-center rounded-md bg-background px-4 py-2 text-sm font-medium cursor-pointer transition-colors hover:bg-accent hover:text-accent-foreground focus-visible:bg-accent focus-visible:text-accent-foreground focus:outline-none disabled:pointer-events-none disabled:opacity-50 disabled:cursor-not-allowed data-[state=open]:text-accent-foreground data-[state=open]:bg-accent/50 data-[state=open]:hover:bg-accent data-[state=open]:focus:bg-accent",
 );
 
 const NavigationMenuTrigger = React.forwardRef<

packages/loopover-ui-kit/src/components/select.tsx

@@ -19,7 +19,7 @@ const SelectTrigger = React.forwardRef<
   <SelectPrimitive.Trigger
     ref={ref}
     className={cn(
-      "flex h-9 w-full items-center justify-between whitespace-nowrap rounded-md border border-input bg-transparent px-3 py-2 text-sm shadow-sm ring-offset-background cursor-pointer data-[placeholder]:text-muted-foreground focus:outline-none focus:ring-1 focus:ring-ring disabled:cursor-not-allowed disabled:opacity-50 [&>span]:line-clamp-1",
+      "flex h-9 w-full items-center justify-between whitespace-nowrap rounded-md border border-input bg-transparent px-3 py-2 text-sm shadow-sm ring-offset-background cursor-pointer data-[placeholder]:text-muted-foreground focus:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 [&>span]:line-clamp-1",
       className,
     )}
     {...props}

packages/loopover-ui-kit/src/components/sheet.tsx

@@ -69,7 +69,7 @@ const SheetContent = React.forwardRef<
       className={cn(sheetVariants({ side }), className)}
       {...props}
     >
-      <SheetPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background cursor-pointer transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-secondary">
+      <SheetPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background cursor-pointer transition-opacity hover:opacity-100 focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-secondary">
         <X className="h-4 w-4" />
         <span className="sr-only">Close</span>
       </SheetPrimitive.Close>