Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/loopover-ui-kit/src/components/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
2 changes: 1 addition & 1 deletion packages/loopover-ui-kit/src/components/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion packages/loopover-ui-kit/src/components/sheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading