From eb47c8951ebe07a074baf0d534f709fbccf2da8e Mon Sep 17 00:00:00 2001
From: RealDiligent <nft.gold.eth@gmail.com>
Date: Fri, 24 Jul 2026 18:38:18 +0800
Subject: [PATCH] fix(ui-kit): move focus ring to focus-visible on
 Select/Dialog/Sheet/NavigationMenu
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .../loopover-ui-kit/src/components/dialog.tsx |  2 +-
 .../focus-visible-convention.test.tsx         | 39 +++++++++++++++++++
 .../src/components/navigation-menu.tsx        |  2 +-
 .../loopover-ui-kit/src/components/select.tsx |  2 +-
 .../loopover-ui-kit/src/components/sheet.tsx  |  2 +-
 5 files changed, 43 insertions(+), 4 deletions(-)
 create mode 100644 packages/loopover-ui-kit/src/components/focus-visible-convention.test.tsx

diff --git a/packages/loopover-ui-kit/src/components/dialog.tsx b/packages/loopover-ui-kit/src/components/dialog.tsx
index 349d688616..e6421777af 100644
--- a/packages/loopover-ui-kit/src/components/dialog.tsx
+++ b/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>
diff --git a/packages/loopover-ui-kit/src/components/focus-visible-convention.test.tsx b/packages/loopover-ui-kit/src/components/focus-visible-convention.test.tsx
new file mode 100644
index 0000000000..e57d1c7874
--- /dev/null
+++ b/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");
+  });
+});
diff --git a/packages/loopover-ui-kit/src/components/navigation-menu.tsx b/packages/loopover-ui-kit/src/components/navigation-menu.tsx
index d845b704f1..045f66f4e1 100644
--- a/packages/loopover-ui-kit/src/components/navigation-menu.tsx
+++ b/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<
diff --git a/packages/loopover-ui-kit/src/components/select.tsx b/packages/loopover-ui-kit/src/components/select.tsx
index 77574540ab..016a77cd75 100644
--- a/packages/loopover-ui-kit/src/components/select.tsx
+++ b/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}
diff --git a/packages/loopover-ui-kit/src/components/sheet.tsx b/packages/loopover-ui-kit/src/components/sheet.tsx
index bc8ed78c4b..b32afabfef 100644
--- a/packages/loopover-ui-kit/src/components/sheet.tsx
+++ b/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>