mirror of
https://github.com/supabase/supabase.git
synced 2026-09-08 19:08:44 +08: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 -->
241 lines
7.3 KiB
TypeScript
241 lines
7.3 KiB
TypeScript
import { IS_PLATFORM } from 'common'
|
|
import { Circle, Code, Minus, Plus, Wind } from 'lucide-react'
|
|
import Link from 'next/link'
|
|
import { useEffect, useMemo, useState } from 'react'
|
|
import { Card, CardContent, CardHeader, CardTitle, cn, Skeleton } from 'ui'
|
|
|
|
import { DiffEditor } from '@/components/ui/DiffEditor'
|
|
import type { EdgeFunctionBodyData } from '@/data/edge-functions/edge-function-body-query'
|
|
import {
|
|
fileKey,
|
|
type EdgeFunctionsDiffResult,
|
|
type FileInfo,
|
|
type FileStatus,
|
|
} from '@/hooks/branches/useEdgeFunctionsDiff'
|
|
import { EMPTY_ARR } from '@/lib/void'
|
|
|
|
const EMPTY_FUNCTION_BODY: EdgeFunctionBodyData = {
|
|
files: EMPTY_ARR,
|
|
}
|
|
|
|
interface EdgeFunctionsDiffPanelProps {
|
|
diffResults: EdgeFunctionsDiffResult
|
|
currentBranchRef?: string
|
|
}
|
|
|
|
interface FunctionDiffProps {
|
|
functionSlug: string
|
|
currentBody: EdgeFunctionBodyData
|
|
mainBody: EdgeFunctionBodyData
|
|
currentBranchRef?: string
|
|
fileInfos: FileInfo[]
|
|
}
|
|
|
|
// Helper to get the status color for file indicators
|
|
const getStatusColor = (status: FileStatus): string => {
|
|
switch (status) {
|
|
case 'added':
|
|
return 'text-brand'
|
|
case 'removed':
|
|
return 'text-destructive'
|
|
case 'modified':
|
|
return 'text-warning'
|
|
case 'unchanged':
|
|
return 'text-muted'
|
|
default:
|
|
return 'text-muted'
|
|
}
|
|
}
|
|
|
|
// Helper to get the status icon for file indicators
|
|
const getStatusIcon = (status: FileStatus) => {
|
|
switch (status) {
|
|
case 'added':
|
|
return Plus
|
|
case 'removed':
|
|
return Minus
|
|
case 'modified':
|
|
return Circle
|
|
case 'unchanged':
|
|
return Circle
|
|
default:
|
|
return Circle
|
|
}
|
|
}
|
|
|
|
const FunctionDiff = ({
|
|
functionSlug,
|
|
currentBody,
|
|
mainBody,
|
|
currentBranchRef,
|
|
fileInfos,
|
|
}: FunctionDiffProps) => {
|
|
// Get all file keys from fileInfos
|
|
const allFileKeys = useMemo(() => fileInfos.map((info) => info.key), [fileInfos])
|
|
|
|
const [activeFileKey, setActiveFileKey] = useState<string | undefined>(() => allFileKeys[0])
|
|
|
|
// Keep active tab in sync when allFileKeys changes (e.g. data fetch completes)
|
|
useEffect(() => {
|
|
if (!activeFileKey || !allFileKeys.includes(activeFileKey)) {
|
|
setActiveFileKey(allFileKeys[0])
|
|
}
|
|
}, [allFileKeys, activeFileKey])
|
|
|
|
const currentFile = currentBody.files.find(
|
|
(f: EdgeFunctionBodyData['files'][number]) => fileKey(f.name) === activeFileKey
|
|
)
|
|
const mainFile = mainBody.files.find(
|
|
(f: EdgeFunctionBodyData['files'][number]) => fileKey(f.name) === activeFileKey
|
|
)
|
|
|
|
const language = useMemo(() => {
|
|
if (!activeFileKey) return 'plaintext'
|
|
if (activeFileKey.endsWith('.ts') || activeFileKey.endsWith('.tsx')) {
|
|
return 'typescript'
|
|
}
|
|
if (activeFileKey.endsWith('.js') || activeFileKey.endsWith('.jsx')) {
|
|
return 'javascript'
|
|
}
|
|
if (activeFileKey.endsWith('.json')) return 'json'
|
|
if (activeFileKey.endsWith('.sql')) return 'sql'
|
|
return 'plaintext'
|
|
}, [activeFileKey])
|
|
|
|
if (allFileKeys.length === 0) return null
|
|
|
|
return (
|
|
<Card>
|
|
<CardHeader>
|
|
<CardTitle>
|
|
<Link
|
|
href={`/project/${currentBranchRef}/functions/${functionSlug}${IS_PLATFORM ? '' : '/details'}`}
|
|
className="flex items-center gap-2"
|
|
>
|
|
<Code strokeWidth={1.5} size={16} className="text-foreground-muted" />
|
|
{functionSlug}
|
|
</Link>
|
|
</CardTitle>
|
|
</CardHeader>
|
|
<CardContent className="p-0 h-96">
|
|
<div className="flex h-full min-h-0">
|
|
<div className="w-48 border-r bg-surface-200 flex flex-col overflow-y-auto">
|
|
<ul className="divide-y divide-border">
|
|
{fileInfos.map((fileInfo) => {
|
|
const Icon = getStatusIcon(fileInfo.status)
|
|
|
|
return (
|
|
<li key={fileInfo.key} className="flex">
|
|
<button
|
|
type="button"
|
|
tabIndex={0}
|
|
onClick={() => setActiveFileKey(fileInfo.key)}
|
|
className={cn(
|
|
'flex-1 text-left text-xs px-4 py-2 flex items-center gap-2',
|
|
activeFileKey === fileInfo.key
|
|
? 'bg-surface-300 text-foreground'
|
|
: 'text-foreground-light hover:bg-surface-300'
|
|
)}
|
|
>
|
|
<Icon
|
|
className={cn('shrink-0', getStatusColor(fileInfo.status))}
|
|
size={12}
|
|
strokeWidth={1}
|
|
/>
|
|
<span className="truncate">{fileInfo.key}</span>
|
|
</button>
|
|
</li>
|
|
)
|
|
})}
|
|
</ul>
|
|
</div>
|
|
<div className="flex-1 min-h-0">
|
|
<DiffEditor
|
|
language={language}
|
|
original={mainFile?.content || ''}
|
|
modified={currentFile?.content || ''}
|
|
options={{ readOnly: true }}
|
|
/>
|
|
</div>
|
|
</div>
|
|
</CardContent>
|
|
</Card>
|
|
)
|
|
}
|
|
|
|
export const EdgeFunctionsDiffPanel = ({
|
|
diffResults,
|
|
currentBranchRef,
|
|
}: EdgeFunctionsDiffPanelProps) => {
|
|
if (diffResults.isLoading) {
|
|
return <Skeleton className="h-64" />
|
|
}
|
|
|
|
const noChanges = diffResults.addedSlugs.length === 0 && diffResults.modifiedSlugs.length === 0
|
|
|
|
if (noChanges) {
|
|
return (
|
|
<div className="p-6 text-center">
|
|
<Wind size={32} strokeWidth={1.5} className="text-foreground-muted mx-auto mb-8" />
|
|
<h3 className="mb-1">No changes detected between branches</h3>
|
|
<p className="text-sm text-foreground-light">
|
|
Any changes to your edge functions will be shown here for review
|
|
</p>
|
|
</div>
|
|
)
|
|
}
|
|
|
|
return (
|
|
<div className="space-y-6">
|
|
{diffResults.addedSlugs.length > 0 && (
|
|
<div>
|
|
<div className="space-y-4">
|
|
{diffResults.addedSlugs.map((slug) => (
|
|
<FunctionDiff
|
|
key={slug}
|
|
functionSlug={slug}
|
|
currentBody={diffResults.addedBodiesMap[slug]!}
|
|
mainBody={EMPTY_FUNCTION_BODY}
|
|
currentBranchRef={currentBranchRef}
|
|
fileInfos={diffResults.functionFileInfo[slug] || EMPTY_ARR}
|
|
/>
|
|
))}
|
|
</div>
|
|
</div>
|
|
)}
|
|
{/* TODO: Removing functions is not supported yet */}
|
|
{/* {diffResults.removedSlugs.length > 0 && (
|
|
<div>
|
|
<div className="space-y-4">
|
|
{diffResults.removedSlugs.map((slug) => (
|
|
<FunctionDiff
|
|
key={slug}
|
|
functionSlug={slug}
|
|
currentBody={EMPTY_ARR}
|
|
mainBody={diffResults.removedBodiesMap[slug]!}
|
|
currentBranchRef={mainBranchRef}
|
|
fileInfos={diffResults.functionFileInfo[slug] || EMPTY_ARR}
|
|
/>
|
|
))}
|
|
</div>
|
|
</div>
|
|
)} */}
|
|
|
|
{diffResults.modifiedSlugs.length > 0 && (
|
|
<div className="space-y-4">
|
|
{diffResults.modifiedSlugs.map((slug) => (
|
|
<FunctionDiff
|
|
key={slug}
|
|
functionSlug={slug}
|
|
currentBody={diffResults.currentBodiesMap[slug]!}
|
|
mainBody={diffResults.mainBodiesMap[slug]!}
|
|
currentBranchRef={currentBranchRef}
|
|
fileInfos={diffResults.functionFileInfo[slug] || EMPTY_ARR}
|
|
/>
|
|
))}
|
|
</div>
|
|
)}
|
|
</div>
|
|
)
|
|
}
|