Skip to content

SelectTrigger, Dialog/Sheet close buttons, and NavigationMenuTrigger use focus: instead of focus-visible:, unlike every other ui-kit primitive #8304

Description

@JSONbored

Context

packages/loopover-ui-kit's interactive primitives consistently use Tailwind's focus-visible: variant (which only applies a focus ring for keyboard/programmatic focus, not mouse-click focus) for their focus ring — see button.tsx (focus-visible:outline-none focus-visible:ring-2 ...), input.tsx, textarea.tsx, switch.tsx, checkbox.tsx, toggle.tsx, slider.tsx, and badge.tsx (focus:outline-none focus-visible:ring-2 ...focus: only clears the native outline, the actual ring is still focus-visible:).

Four components break this convention and instead apply their ring/highlight on plain focus:, which also fires on mouse-click focus and produces a persistent focus ring/background highlight after every click — something none of the other primitives do:

  • select.tsx:22SelectTrigger: ... focus:outline-none focus:ring-1 focus:ring-ring ...
  • dialog.tsx:47DialogPrimitive.Close: ... focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 ...
  • sheet.tsx:72SheetPrimitive.Close: ... focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 ...
  • navigation-menu.tsx:44NavigationMenuTrigger's trigger style: ... focus:bg-accent focus:text-accent-foreground focus:outline-none ...

Requirements

  • In each of the four locations above, change the ring/background-highlight classes from focus: to focus-visible:, matching the exact convention already used by button.tsx/input.tsx/textarea.tsx/switch.tsx/checkbox.tsx/toggle.tsx/slider.tsx.
  • Keep focus:outline-none as-is where present (clearing the native browser outline on any focus, not just keyboard focus, is correct and matches every other primitive) — only the visible ring/highlight classes (focus:ring-*, focus:bg-accent, focus:text-accent-foreground) need to move to focus-visible:.
  • Do not touch any other className on these four elements (radius, padding, color tokens unrelated to focus).
  • This is a class-rename-only fix — no new abstraction, no adoption of the .focus-ring CSS utility in theme.css (that utility is for app-level nav/hero elements per its own comment, not the primitive-component convention these four should follow).

Deliverables

  • select.tsx's SelectTrigger ring classes changed from focus: to focus-visible:
  • dialog.tsx's DialogPrimitive.Close ring classes changed from focus: to focus-visible:
  • sheet.tsx's SheetPrimitive.Close ring classes changed from focus: to focus-visible:
  • navigation-menu.tsx's NavigationMenuTrigger highlight classes changed from focus: to focus-visible:
  • A regression test (or extension of an existing render test) asserting at least one of these four components no longer carries a bare focus:ring/focus:bg-accent class in its rendered class list

Test Coverage Requirements

packages/loopover-ui-kit is not in the root vitest.config.ts's coverage.include and is not Codecov-gated (its own vitest.config.ts states the acceptance signal is the suite running and passing, not a percentage) — still add a test per the Deliverables above so the fix is verifiable and doesn't silently regress.

Expected Outcome

SelectTrigger, DialogPrimitive.Close, SheetPrimitive.Close, and NavigationMenuTrigger only show their focus ring/highlight for keyboard/programmatic focus, matching every other interactive primitive in @loopover/ui-kit — clicking them with a mouse no longer leaves a lingering ring/highlight.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is neededvisualUI/web visual work — owner-led, NOT for Gittensor contributors (extensions excepted)

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions