mirror of
https://github.com/supabase/supabase.git
synced 2026-10-06 01:45:10 +03:00
## What kind of change does this PR introduce? A11y cleanup follow-up to #47984 / [DEPR-626](https://linear.app/supabase/issue/DEPR-626). ## What is the current behavior? Studio had 82 ratcheted `supabase/require-explicit-tabindex` violations (raw `<button>` / `role="button"` without explicit `tabIndex`). ## What is the new behavior? - Explicit `tabIndex={0}` (or disabled → `-1`) on those Studio call sites across nav, `components/ui`, Database, Storage, and the remainder - Ratchet baseline cleared (**82 → 0**) and the rule **removed from the Studio ratchet** (debt is gone; ratchet is temporary) - Rule remains a shared **`warn`** for now — promoting to `error` (and sweeping www/docs/design-system) is a follow-up - Also fixed the learn/ui-library call sites that surfaced while experimenting with error promotion - Small follow-ups where making controls focusable exposed gaps: accessible names, disabled/focus consistency, focus-ring polish on To-test surfaces, home section `KeyboardSensor`, and an E2E locator tightened after `aria-label="Remove column"` Prefer migrating to `Button` from `ui` in future touch-ups; this PR takes the minimal path so Studio debt can stay at zero. ## Additional context Batches landed together so baseline conflicts stayed simple while chipping away: - Hotspots / nav (FirstLevelNav, Marketplace, AttachmentUpload, Column, Tabs, …) - `components/ui` shared - Database + Storage - Remainder **Out of scope / intentional deferrals** - Promoting `supabase/require-explicit-tabindex` to a lint **error** (follow-up after www/docs/design-system sweeps) - Tabs/Radio roving, tooltips, context menus, in-menu items - Full keyboard-accessible tab-close UX (close stays hover + `tabIndex={-1}`; context menu still closes tabs) - Data API docs links (`/project/<ref>/api` redirect) **Reviewer notes** - Rule only flags raw `<button>` / `role="button"` without a `tabIndex` prop. `Button` from `ui` already bakes this in - `tabIndex={-1}` is intentional for disabled controls, in-menu / roving-focus children, and hover-only tab close - For dnd-kit grips, put `tabIndex` **after** `{...attributes}` so it isn’t overwritten (TS2783) ### To test Use **Safari** with macOS Keyboard navigation **off** (System Settings → Keyboard). Chrome once for a sanity pass. For each surface below: Tab until the control is focused, then activate with Enter/Space where relevant. 1. **API Docs side panel** (Table Editor → open a table → **API docs**) - Floating API Docs panel — **not** `/project/<ref>/api` (that redirects to Data API docs; language ToggleGroup uses arrow keys; links are out of scope) - Left nav buttons — Tab through several and activate one; active highlight / navigation still works 2. **Integrations → Marketplace** - Enable **Integrations layout** feature preview first (avatar menu → Feature previews) - `/org/<slug>/integrations` or project integrations marketplace - “Clear all”, grid/list toggles — Tab + activate 3. **Table Editor → create a table → Columns** - Drag handles only appear while **creating** (not when editing an existing table) - Tab to grip / remove (X) / sensitive-data eye if shown 4. **Project Home** — section drag handles - Tab to a grip (visible focus ring) - Optional: Space to pick up, arrows to move, Space/Esc to drop (KeyboardSensor added) - Mouse dnd still works 5. **Storage → Policies** — expand/collapse bucket list chevron (design-system focus ring, no stuck grey open bg) 6. **Support form** (Help → Support) — attachment remove (×) and add-attachment control when visible Disabled controls should be **skipped** by Tab. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Accessibility Improvements** * Improved keyboard navigation throughout Studio by explicitly managing focus (`tabIndex`) across many interactive controls (menus, tabs, tables, charts, dialogs, navigation, and form actions). * Disabled or non-interactive controls are now removed from the tab order (or made unfocusable), while available actions remain reachable. * Ensured `type="button"` on relevant controls to prevent unintended submissions, and refined keyboard focus behavior for various toggles and copy/remove actions. * **Chores** * Updated the ESLint rule baseline configuration to match the new focus behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
170 lines
5.2 KiB
TypeScript
170 lines
5.2 KiB
TypeScript
'use client'
|
|
|
|
import { ChevronRight, Lock } from 'lucide-react'
|
|
import Link, { LinkProps } from 'next/link'
|
|
import { usePathname } from 'next/navigation'
|
|
import React, { useEffect, useState } from 'react'
|
|
import { Badge, cn } from 'ui'
|
|
|
|
import { useMobileMenu } from '@/hooks/use-mobile-menu'
|
|
import { SidebarNavItem } from '@/types/nav'
|
|
|
|
// We extend:
|
|
// 1. LinkProps - for Next.js Link component props (prefetch, etc)
|
|
// 2. AnchorHTMLAttributes - for standard HTML anchor props (className, etc)
|
|
// We omit href from both since we compute it internally from item.href
|
|
interface NavigationItemProps
|
|
extends Omit<LinkProps, 'href'>, Omit<React.AnchorHTMLAttributes<HTMLAnchorElement>, 'href'> {
|
|
item: SidebarNavItem
|
|
onClick?: (e: React.MouseEvent<HTMLAnchorElement>) => void
|
|
level?: number
|
|
internalPaths?: string[]
|
|
isLoggedIn?: boolean
|
|
}
|
|
|
|
const NavigationItem: React.FC<NavigationItemProps> = ({
|
|
item,
|
|
onClick,
|
|
level = 0,
|
|
internalPaths,
|
|
isLoggedIn = true,
|
|
...props
|
|
}) => {
|
|
const { setOpen } = useMobileMenu()
|
|
const pathname = usePathname()
|
|
const [isOpen, setIsOpen] = useState(false)
|
|
|
|
const pathParts = pathname.split('/')
|
|
const slug = pathParts[pathParts.length - 1]
|
|
|
|
const hasChildren = item.items && item.items.length > 0
|
|
|
|
// Check if internal content exists for this item and user is logged in
|
|
const hasInternal = item.href && internalPaths?.includes(item.href) && isLoggedIn
|
|
|
|
// Auto-expand if any child is active
|
|
useEffect(() => {
|
|
if (hasChildren) {
|
|
const hasActiveChild = item.items?.some((child) => {
|
|
return pathname === child.href
|
|
})
|
|
if (hasActiveChild) {
|
|
setIsOpen(true)
|
|
}
|
|
}
|
|
}, [hasChildren, item.items, pathname])
|
|
|
|
// Use item.href if available, otherwise build from slug
|
|
let href = item.href
|
|
if (!href && slug) {
|
|
href = `/docs/${slug}`
|
|
}
|
|
|
|
// Determine if this link represents the current page
|
|
const isActive = pathname === href
|
|
|
|
const handleLinkClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
|
|
// Close the mobile menu when navigating
|
|
setOpen(false)
|
|
|
|
// Call the onClick prop if it exists
|
|
if (onClick) {
|
|
onClick(e)
|
|
}
|
|
}
|
|
|
|
const handleButtonClick = (e: React.MouseEvent<HTMLButtonElement>) => {
|
|
e.preventDefault()
|
|
setIsOpen(!isOpen)
|
|
}
|
|
|
|
const itemClasses = cn(
|
|
'flex text-sm rounded-md transition-colors',
|
|
level === 0 ? 'px-3 py-2' : 'px-3 py-1.5',
|
|
isActive
|
|
? 'bg-surface-200 text-foreground'
|
|
: hasChildren && isOpen
|
|
? 'bg-surface-100 text-foreground'
|
|
: 'text-foreground-lighter hover:bg-surface-100 hover:text-foreground'
|
|
)
|
|
|
|
return (
|
|
<li>
|
|
{hasChildren ? (
|
|
<>
|
|
<button
|
|
type="button"
|
|
tabIndex={0}
|
|
onClick={handleButtonClick}
|
|
className={cn('w-full flex items-center justify-between gap-2 zans', itemClasses)}
|
|
>
|
|
<span className="flex items-center gap-2 flex-1 min-w-0">
|
|
{item.title}
|
|
{item.new && (
|
|
<Badge variant="default" className="capitalize shrink-0">
|
|
NEW
|
|
</Badge>
|
|
)}
|
|
</span>
|
|
<ChevronRight
|
|
className={cn(
|
|
'w-4 h-4 transition-transform shrink-0',
|
|
isOpen && 'rotate-90',
|
|
(hasChildren && isOpen) || isActive ? 'text-foreground' : 'text-foreground-lighter'
|
|
)}
|
|
/>
|
|
</button>
|
|
{isOpen && (
|
|
<ul className="mt-1 ml-3 space-y-1 border-l border-border pl-3">
|
|
{item.items?.map((childItem, i) => (
|
|
<NavigationItem
|
|
item={childItem}
|
|
key={`${childItem.href}-${i}`}
|
|
level={level + 1}
|
|
internalPaths={internalPaths}
|
|
isLoggedIn={isLoggedIn}
|
|
/>
|
|
))}
|
|
</ul>
|
|
)}
|
|
</>
|
|
) : (
|
|
<>
|
|
<Link href={href || '#'} {...props} onClick={handleLinkClick} className={itemClasses}>
|
|
<span className="flex items-center gap-2">
|
|
<span className="truncate">{item.title}</span>
|
|
{item.new && (
|
|
<Badge variant="default" className="capitalize shrink-0">
|
|
NEW
|
|
</Badge>
|
|
)}
|
|
</span>
|
|
</Link>
|
|
{hasInternal && (
|
|
<Link
|
|
href={`/internal${href}`}
|
|
onClick={handleLinkClick}
|
|
className={cn(
|
|
'flex text-sm rounded-md transition-colors mt-1',
|
|
level === 0 ? 'px-3 py-2' : 'px-3 py-1.5',
|
|
pathname === `/internal${href}`
|
|
? 'bg-surface-200 text-foreground'
|
|
: 'text-foreground-lighter hover:bg-surface-100 hover:text-foreground'
|
|
)}
|
|
>
|
|
<span className="flex items-center gap-2">
|
|
<Lock className="w-3 h-3 text-foreground-muted shrink-0" />
|
|
<span className="truncate">{item.title} (Internal)</span>
|
|
</span>
|
|
</Link>
|
|
)}
|
|
</>
|
|
)}
|
|
</li>
|
|
)
|
|
}
|
|
|
|
NavigationItem.displayName = 'NavigationItem'
|
|
|
|
export default NavigationItem
|