fix(ui): standardise disclosure focus rings (#49660)

## What kind of change does this PR introduce? Bug fix and design-system accessibility update. ## What is the current behavior? Accordion and collapsible triggers do not consistently use the shared focus treatment. Accordion focus can animate from the browser outline, while call sites need to manage radius and focus geometry themselves. ## What is the new behavior? Bare accordion and collapsible triggers use the rounded `focus-inset` treatment by default, positioned 2px inside the control. Interactive children composed with `asChild`, such as a standard `Button`, retain their own focus treatment. Accordion triggers include a small default horizontal hit-area padding (`px-2`) and retain an explicit outer-ring option for standalone controls. The focus indicator is not animated. Focused tests cover the default and opt-out accordion variants. | Before | After | | --- | --- | | <img width="1792" height="384" alt="CleanShot 2026-09-07 at 15 40 23@2x" src="https://github.com/user-attachments/assets/8c8fd99a-3669-465a-a7b2-d272ec92dec7" /> | <img width="1784" height="498" alt="CleanShot 2026-09-07 at 15 39 52@2x" src="https://github.com/user-attachments/assets/7a0b7de1-a4bc-48cf-b9fc-3421618806b9" /> | | <img width="1564" height="360" alt="CleanShot 2026-09-07 at 15 35 11@2x" src="https://github.com/user-attachments/assets/34945180-e671-4dc2-981a-4c683d08f221" /> | <img width="1552" height="378" alt="CleanShot 2026-09-07 at 15 36 19@2x" src="https://github.com/user-attachments/assets/09f2c030-9ffb-4fa5-bf63-5e172326e8ef" /> | ## Visual impact Most call sites should look the same at rest. The exceptions are accordion triggers that did not already set their own horizontal padding: they pick up the new default `px-2`, `rounded-md`, and lose the old blanket `transition-all` on the trigger (chevron rotation is unchanged). | Resting layout change | Why | | --- | --- | | Accordion triggers without an explicit `px-*` | Inherit new default `px-2` and `rounded-md` | | Accordion triggers with their own `px-*` | Unchanged (call-site padding wins) | | Collapsible triggers | Focus ring only; spacing comes from the child element | | Direct `focus-inset` users (table rows, etc.) | Focus ring only | ## To test Use Tab to focus each disclosure trigger. Confirm the focus outline is rounded, sits 2px inside the trigger, and appears immediately without a dark flash. Previews: [design-system](https://design-system-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app) · [studio-staging](https://studio-staging-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app) · [docs](https://docs-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app) · [www](https://zone-www-dot-com-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app) ### Design system - [Accordion](https://design-system-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/design-system/docs/components/accordion) - [Collapsible](https://design-system-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/design-system/docs/components/collapsible) ### Studio (focus-only unless noted) - [Account → Access tokens → **New token**](https://studio-staging-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/dashboard/account/tokens): permission category accordions (already `px-4`) - [Database → Roles](https://studio-staging-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/dashboard/project/_/database/roles): role row collapsibles - [Observability → Query performance](https://studio-staging-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/dashboard/project/_/observability/query-performance): index suggestion accordions (already `px-4`) - [Settings → JWT (legacy)](https://studio-staging-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/dashboard/project/_/settings/jwt/legacy): migration help collapsible ### Docs - Docs mobile nav (narrow viewport): accordion sections **(resting layout: +px-2)** ### WWW - [Pricing → FAQs](https://zone-www-dot-com-git-dnywh-fixaccordion-focus-rings-supabase.vercel.app/pricing): FAQ accordions **(resting layout: +px-2)** - WWW mobile nav (narrow viewport): menu accordions **(resting layout: +px-2)** ### Not worth opening manually Sheets, modals, error fallbacks, and replication/cron/integration advanced-settings accordions inherit the focus fix but need specific flows or failure states to reach. Error-state troubleshooting accordions (`TroubleshootingSections`) also gain **+px-2** when visible.  ## Summary by CodeRabbit ## New Features - Accordion triggers now use an inset focus style by default, with an option to use the standard ring style. - Accordion and collapsible trigger styling can be customized more flexibly. ## Accessibility - Focus indicators appear consistently without flashing browser defaults. - Focus transitions are limited to visual properties that preserve immediate keyboard focus visibility. ## Documentation - Updated accessibility guidance explains inset focus behavior and transition considerations.
2026-10-05 09:25:06 +03:00 · 2026-09-07 16:59:40 +10:00
1 parent c365549ab8
commit 9f5b5ea6a7
@@ -59,16 +59,17 @@ focus-visible:ring-offset-background
 Uses `outline` (not `ring`) so it paints reliably on interactive `<tr>`s. Tailwind `ring` is `box-shadow`, which browsers often skip on `display: table-row` (notably Safari). Do not put `focus-ring` or raw `ring-*` on a `<tr>`, and do not add `outline-hidden` alongside `focus-inset`. `outline-hidden` sets `outline-style: none` and will hide the indicator.

 ```txt
+outline: 2px solid transparent
+outline-offset: -2px
+transition-property: color, background-color, border-color, ...
+
 &:focus-visible {
-  outline-style: solid
-  outline-width: 2px
-  outline-offset: -2px
  outline-color: var(--ring)
  border-radius: var(--radius-md)
 }
 ```

-`outline-hidden` is always on (not `focus-visible:`-prefixed) so mouse click does not show the browser’s default outline; the focus indicator replaces it for keyboard focus only.
+`focus-ring` keeps `outline-hidden` always on so mouse clicks do not show the browser’s default outline. `focus-inset` reserves a transparent outline instead. Its transition property list deliberately excludes outline properties so the keyboard focus indicator appears immediately, even when a call site uses `transition-all`.

 Rules:

@@ -198,10 +198,15 @@
 }

@utility focus-inset {
+  /* Reserve outline geometry up front so focus never flashes the browser default. */
+  outline: 2px solid transparent;
+  outline-offset: -2px;
+  /* Call sites often add `transition`/`transition-all`; keep outline instant. */
+  transition-property:
+    color, background-color, border-color, text-decoration-color, fill, stroke, opacity, transform,
+    filter, backdrop-filter, box-shadow;
+
  &:focus-visible {
-    outline-style: solid;
-    outline-width: 2px;
-    outline-offset: -2px;
    outline-color: var(--ring);
    border-radius: var(--radius-md, 0.375rem);
  }
@@ -0,0 +1,37 @@
+import { render, screen } from '@testing-library/react'
+import { describe, expect, it } from 'vitest'
+
+import { Accordion, AccordionItem, AccordionTrigger } from './accordion'
+
+describe('AccordionTrigger', () => {
+  it('provides rounded geometry and the shared inset focus ring', () => {
+    render(
+      <Accordion type="single" collapsible>
+        <AccordionItem value="advanced">
+          <AccordionTrigger>Advanced settings</AccordionTrigger>
+        </AccordionItem>
+      </Accordion>
+    )
+
+    const trigger = screen.getByRole('button', { name: 'Advanced settings' })
+    expect(trigger).toHaveClass('px-2', 'relative', 'focus-inset')
+    expect(trigger).not.toHaveClass('rounded-md')
+    expect(trigger).not.toHaveClass('focus-ring')
+    expect(trigger).not.toHaveClass('transition-colors')
+    expect(trigger).not.toHaveClass('transition-all')
+  })
+
+  it('supports an outer focus ring when explicitly requested', () => {
+    render(
+      <Accordion type="single" collapsible>
+        <AccordionItem value="advanced">
+          <AccordionTrigger focusVariant="ring">Advanced settings</AccordionTrigger>
+        </AccordionItem>
+      </Accordion>
+    )
+
+    const trigger = screen.getByRole('button', { name: 'Advanced settings' })
+    expect(trigger).toHaveClass('rounded-md', 'focus-ring')
+    expect(trigger).not.toHaveClass('focus-inset')
+  })
+})
@@ -19,41 +19,51 @@ AccordionItem.displayName = 'AccordionItem'

 const AccordionTrigger = React.forwardRef<
  React.ElementRef<typeof AccordionPrimitive.Trigger>,
-  React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & { hideIcon?: boolean }
->(({ className, children, hideIcon, disabled, tabIndex, ...props }, ref) => {
-  const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)
+  React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & {
+    focusVariant?: 'ring' | 'inset'
+    hideIcon?: boolean
+  }
+>(
+  (
+    { className, children, focusVariant = 'inset', hideIcon, disabled, tabIndex, ...props },
+    ref
+  ) => {
+    const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)

-  return (
-    <AccordionPrimitive.Header asChild>
-      <div className="flex">
-        <AccordionPrimitive.Trigger
-          ref={ref}
-          className={cn(
-            'cursor-pointer flex flex-1 gap-2 items-center justify-between py-4 text-left',
-            'font-medium transition-all hover:underline',
-            '[&[data-state=open]>svg]:rotate-180',
-            className
-          )}
-          {...props}
-          disabled={disabled}
-          tabIndex={computedTabIndex}
-        >
-          {children}
-          {!hideIcon && (
-            <ChevronDown
-              aria-hidden="true"
-              className={cn(
-                'h-4 w-4 shrink-0',
-                'transition-transform duration-200',
-                'motion-reduce:transition-none motion-reduce:duration-0'
-              )}
-            />
-          )}
-        </AccordionPrimitive.Trigger>
-      </div>
-    </AccordionPrimitive.Header>
-  )
-})
+    return (
+      <AccordionPrimitive.Header asChild>
+        <div className="flex">
+          <AccordionPrimitive.Trigger
+            ref={ref}
+            className={cn(
+              'cursor-pointer flex flex-1 gap-2 items-center justify-between px-2 py-4 text-left',
+              'font-medium hover:underline',
+              '[&[data-state=open]>svg]:rotate-180',
+              focusVariant === 'ring' && 'rounded-md',
+              className,
+              focusVariant === 'ring' ? 'focus-ring' : 'relative focus-inset'
+            )}
+            {...props}
+            disabled={disabled}
+            tabIndex={computedTabIndex}
+          >
+            {children}
+            {!hideIcon && (
+              <ChevronDown
+                aria-hidden="true"
+                className={cn(
+                  'h-4 w-4 shrink-0',
+                  'transition-transform duration-200',
+                  'motion-reduce:transition-none motion-reduce:duration-0'
+                )}
+              />
+            )}
+          </AccordionPrimitive.Trigger>
+        </div>
+      </AccordionPrimitive.Header>
+    )
+  }
+)
 AccordionTrigger.displayName = AccordionPrimitive.Trigger.displayName

 const AccordionContent = React.forwardRef<
@@ -3,6 +3,7 @@
 import { Collapsible as CollapsiblePrimitive } from 'radix-ui'
 import * as React from 'react'

+import { cn } from '../../../lib/utils/cn'
 import { getExplicitTabIndex } from '../../../lib/utils/getExplicitTabIndex'

 const Collapsible = CollapsiblePrimitive.Root
@@ -10,12 +11,13 @@ const Collapsible = CollapsiblePrimitive.Root
 const CollapsibleTrigger = React.forwardRef<
  React.ElementRef<typeof CollapsiblePrimitive.CollapsibleTrigger>,
  React.ComponentPropsWithoutRef<typeof CollapsiblePrimitive.CollapsibleTrigger>
->(({ disabled, tabIndex, ...props }, ref) => {
+>(({ className, disabled, tabIndex, ...props }, ref) => {
  const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)

  return (
    <CollapsiblePrimitive.CollapsibleTrigger
      ref={ref}
+      className={cn('relative focus-inset', className)}
      {...props}
      disabled={disabled}
      tabIndex={computedTabIndex}