From e57aae3c837f42c16831e221b6401bbb36854f6b Mon Sep 17 00:00:00 2001 From: Danny White <3104761+dnywh@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:52:21 +1000 Subject: [PATCH] feat(design-system): document disabled controls and add focusableWhenDisabled (#50068) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What kind of change does this PR introduce? Docs update, with supporting `ui` and Studio changes. ## What is the current behaviour? Disabled buttons with tooltips use native `disabled`, which removes them from the tab order. Keyboard users cannot focus the control or read the tooltip explaining why an action is blocked. The design system also lacked guidance on keeping disabled actions discoverable and explaining why they are unavailable. ## What is the new behaviour? - Adds a **Disabled controls** section to the accessibility docs, with live examples for a focusable disabled button and visible page-level context - Adds `focusableWhenDisabled` to `Button`, keeping `disabled` as the semantic state while using `aria-disabled`, retaining keyboard focus, and guarding click handlers - Updates Studio's `ButtonTooltip` to make disabled buttons with tooltip text focusable automatically Also includes earlier design-system fixes on this branch: - Centralises `BASE_PATH` with a `/design-system` fallback so asset URLs work without a local `.env` file - Fixes sidebar hover and active tokens in design-system and ui-library, aligned with Studio's `InnerSideMenuItem` ## To test **Design system** 1. Open the [accessibility preview](https://design-system-git-fix-design-system-docs-and-nav-fixes-supabase.vercel.app/design-system/docs/accessibility) 2. Scroll to **Disabled controls** 3. Tab to the **disabled-focusable** example. Confirm the button remains focusable, looks disabled, and shows its tooltip on focus 4. Confirm the **disabled-unavailable-with-notice** example shows the admonition and focusable disabled button pattern **Studio (optional, requires a High Availability project)** 5. Go to Settings → General → **Pause project**. Tab to the button and confirm it remains focusable, looks disabled, and shows the HA tooltip on focus 6. Go to Database → Backups and find **Restore** on a scheduled backup row. Confirm the same behaviour --- apps/design-system/app/layout.tsx | 6 +- .../components/homepage-svg-handler.tsx | 4 +- .../components/side-navigation-item.tsx | 9 +-- .../components/theme-settings.tsx | 4 +- .../content/docs/accessibility.mdx | 48 ++++++++++++- .../content/docs/components/button.mdx | 18 ++++- apps/design-system/lib/constants.ts | 3 + .../connect-interstitial-logo-uploaded.tsx | 3 +- .../default/example/disabled-focusable.tsx | 26 +++++++ .../disabled-unavailable-with-notice.tsx | 50 ++++++++++++++ .../radio-group-card-with-children.tsx | 4 +- apps/design-system/registry/examples.ts | 12 ++++ .../interfaces/Compute/ComputeList.test.tsx | 4 +- .../Database/Backups/BackupItem.test.tsx | 5 +- .../__tests__/ExplorerNotebookTab.test.tsx | 2 +- .../Explorer/__tests__/QueryTab.test.tsx | 2 +- .../InvoicesSettings.test.tsx | 2 +- .../PauseProjectButton.test.tsx | 5 +- apps/studio/components/ui/ButtonTooltip.tsx | 20 ++++-- .../App/IndirectTaxDeclarationModal.test.tsx | 2 +- apps/studio/tests/lib/disabled-matchers.ts | 25 +++++++ apps/studio/tests/vitestSetup.ts | 1 + .../components/side-navigation-item.tsx | 7 +- .../ui/src/components/Button/Button.test.tsx | 69 ++++++++++++++++++- packages/ui/src/components/Button/Button.tsx | 61 ++++++++++++---- 25 files changed, 342 insertions(+), 50 deletions(-) create mode 100644 apps/design-system/lib/constants.ts create mode 100644 apps/design-system/registry/default/example/disabled-focusable.tsx create mode 100644 apps/design-system/registry/default/example/disabled-unavailable-with-notice.tsx create mode 100644 apps/studio/tests/lib/disabled-matchers.ts diff --git a/apps/design-system/app/layout.tsx b/apps/design-system/app/layout.tsx index de7f2407868..31c70dfced9 100644 --- a/apps/design-system/app/layout.tsx +++ b/apps/design-system/app/layout.tsx @@ -1,18 +1,16 @@ import 'react-data-grid/lib/styles.css' import '@/styles/globals.css' -import type { Metadata, Viewport } from 'next' - import { genFaviconData } from 'common/MetaFavicons/app-router' +import type { Metadata, Viewport } from 'next' import { Providers } from './Providers' import { Toaster } from './toaster' +import { BASE_PATH } from '@/lib/constants' import { inter, manrope, sourceCodePro } from '@/lib/fonts' const className = `${inter.variable} ${manrope.variable} ${sourceCodePro.variable}` -const BASE_PATH = process.env.NEXT_PUBLIC_BASE_PATH || '/design-system' - export const metadata: Metadata = { applicationName: 'Supabase Design System', title: 'Supabase Design System', diff --git a/apps/design-system/components/homepage-svg-handler.tsx b/apps/design-system/components/homepage-svg-handler.tsx index fef1ad5c3f4..6fe1b0c3133 100644 --- a/apps/design-system/components/homepage-svg-handler.tsx +++ b/apps/design-system/components/homepage-svg-handler.tsx @@ -4,6 +4,8 @@ import { useTheme } from 'next-themes' import SVG from 'react-inlinesvg' import { cn } from 'ui' +import { BASE_PATH } from '@/lib/constants' + const HomepageSvgHandler = ({ name, className }: { name: string; className?: string }) => { const { resolvedTheme } = useTheme() @@ -11,7 +13,7 @@ const HomepageSvgHandler = ({ name, className }: { name: string; className?: str
) diff --git a/apps/design-system/components/side-navigation-item.tsx b/apps/design-system/components/side-navigation-item.tsx index c0932c8e575..491905452e1 100644 --- a/apps/design-system/components/side-navigation-item.tsx +++ b/apps/design-system/components/side-navigation-item.tsx @@ -28,10 +28,11 @@ export const NavigationItem: React.FC<{ item: SidebarNavItem }> = React.memo(({ 'items-center', 'h-6', 'text-sm', - 'text-foreground-lighter px-6', - !isActive && 'hover:bg-surface-100 hover:text-foreground', - isActive && 'bg-surface-200 text-foreground', - 'transition-all' + 'px-6', + 'transition-all', + isActive + ? 'bg-selection text-foreground' + : 'text-foreground-light hover:bg-surface-200 hover:text-foreground' )} >
{ const [mounted, setMounted] = useState(false) const { theme, setTheme } = useTheme() @@ -35,7 +37,7 @@ const ThemeSettings = () => { > {singleThemes.map((theme) => ( - + ))} diff --git a/apps/design-system/content/docs/accessibility.mdx b/apps/design-system/content/docs/accessibility.mdx index c1660c5b963..4ea7efcfafc 100644 --- a/apps/design-system/content/docs/accessibility.mdx +++ b/apps/design-system/content/docs/accessibility.mdx @@ -15,13 +15,14 @@ Accessibility is about making an interface work for as many people as possible a About to push some code? At a minimum, check your work against this list: - Are interactive page elements [keyboard-focusable](#focus-management)? +- Are unavailable actions [discoverable and explained](#disabled-controls) for keyboard users? - Are all elements announcable by a [screen reader](#screen-reader-support)? - Are textual elements legible and scalable? - Can I use this on a smaller and/or older device? ## Focus management -All interactive page elements should be reachable by keyboard. Given the below inconsistency between devices and browsers, add `tabIndex={0}` to all buttons, links, and non-text inputs, ideally at the component level. Consider tying the state of `tabIndex` to the `disabled` state of a component, if applicable. +All interactive page elements should be reachable by keyboard. Native buttons, links with `href`, and form inputs are keyboard accessible by default. Add `tabIndex={0}` only to bespoke interactive elements. For controls using native `disabled`, tie `tabIndex` to that state (disabled controls default to `tabIndex={-1}`). Controls that use `aria-disabled` to stay discoverable should remain at `tabIndex={0}`. See [Disabled controls](#disabled-controls). Chromium-based browsers and Firefox handle this automatically via the Tab key. Safari, by default, requires the Option key to also be held down. Enabling _Keyboard navigation_ on macOS Settings [removes this requirement](https://mayank.co/blog/safari-focus/#keyboard-navigation) but makes links non-tabbable as a result. @@ -125,6 +126,51 @@ Some keyboard-navigable content may contain hundreds or thousands of items. Help Apps with persistent header and sidebar chrome should expose a skip link as the first focusable element. Use the shared [Skip to Content](fragments/skip-to-content) fragment which owns the component API, usage sample, and target landmark contract. +## Disabled controls + +Native `disabled` controls are removed from the tab order. When users need to focus a disabled button to discover the action or understand why it is unavailable, add `focusableWhenDisabled`. + +### Focusable when disabled + +Use `disabled` with `focusableWhenDisabled` when an action is unavailable for a reason that is not obvious, especially when you show a tooltip explaining why: + +- Permission gates +- Plan or infrastructure restrictions +- Business rules that block an otherwise visible action + +`focusableWhenDisabled` changes how the disabled state is implemented. It sets `aria-disabled="true"`, keeps the control in the tab order, and applies disabled styling without `pointer-events-none`. Guard handlers are built into [Button](components/button). See also [MDN: aria-disabled](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-disabled). + +Tab to the example below with the keyboard. The disabled button stays focusable and exposes its tooltip. + + + +Implementation checklist for focusable disabled buttons: + +- Add `focusableWhenDisabled` when the reason for disabling the control is not obvious +- Pair with a tooltip when you need to explain why +- Guard `onClick` and keyboard activation (`Enter` / `Space`) (`Button` does this automatically) +- Keep `tabIndex={0}` (`Button` does this automatically) +- Do **not** use `pointer-events-none` on the control (it blocks hover and tooltips) + +```tsx showLineNumbers + + + + + {unavailable && {reason}} + +``` + +In Studio, `ButtonTooltip` adds `focusableWhenDisabled` to disabled buttons with tooltip text automatically. + +### Page-level context + +Tooltips alone are not enough for significant restrictions. Pair focusable disabled controls with visible page context (for example: an [Admonition](fragments/admonition), empty state, or inline copy) so the reason is available even without hover or focus. + + + ## Screen readers Textual elements are supported out-of-the-box by screen readers. diff --git a/apps/design-system/content/docs/components/button.mdx b/apps/design-system/content/docs/components/button.mdx index 6a89781e033..3af2a64e17f 100644 --- a/apps/design-system/content/docs/components/button.mdx +++ b/apps/design-system/content/docs/components/button.mdx @@ -138,8 +138,22 @@ Inside [Admonition](../fragments/admonition#split-button-with-dropdown) actions [Keyboard focus](../accessibility#focus-management) is automatically handled: - Enabled buttons default to `tabIndex={0}` (keyboard accessible) -- Disabled buttons default to `tabIndex={-1}` (removed from tab order) +- Buttons with native `disabled` default to `tabIndex={-1}` (removed from tab order) - You can still override with an explicit `tabIndex` prop when needed - Keyboard focus uses the shared `focus-ring` utility; variants do not change ring colour -You therefore don't need to manually set `tabIndex`, as Button handles it automatically based on its `disabled` state. +You therefore don't need to manually set `tabIndex` for buttons using native `disabled`. + +When a disabled action has a non-obvious reason and needs a tooltip, add `focusableWhenDisabled` so keyboard users can still focus the control. See [Disabled controls](../accessibility#disabled-controls). + +### Focusable when disabled + +Use `focusableWhenDisabled` with `disabled` when the action is blocked for a non-obvious reason and you need a tooltip or other explanation. The control stays in the tab order and uses `aria-disabled` instead of native `disabled`. + +```tsx + +``` + +In Studio, `ButtonTooltip` adds `focusableWhenDisabled` to disabled buttons with tooltip text automatically. diff --git a/apps/design-system/lib/constants.ts b/apps/design-system/lib/constants.ts new file mode 100644 index 00000000000..abbdb2aade6 --- /dev/null +++ b/apps/design-system/lib/constants.ts @@ -0,0 +1,3 @@ +const rawBasePath = process.env.NEXT_PUBLIC_BASE_PATH || 'design-system' + +export const BASE_PATH = rawBasePath.startsWith('/') ? rawBasePath : `/${rawBasePath}` diff --git a/apps/design-system/registry/default/example/connect-interstitial-logo-uploaded.tsx b/apps/design-system/registry/default/example/connect-interstitial-logo-uploaded.tsx index 5aaf43fab02..d02833a6006 100644 --- a/apps/design-system/registry/default/example/connect-interstitial-logo-uploaded.tsx +++ b/apps/design-system/registry/default/example/connect-interstitial-logo-uploaded.tsx @@ -8,6 +8,7 @@ import { SignOutButton, SupabaseLogo, } from './connect-interstitial-shared' +import { BASE_PATH } from '@/lib/constants' /** Stand-in uploaded OAuth icon: checked-in solid-colour bitmap (not a real brand). */ function UploadedAppLogo() { @@ -15,7 +16,7 @@ function UploadedAppLogo() { Acme diff --git a/apps/design-system/registry/default/example/disabled-focusable.tsx b/apps/design-system/registry/default/example/disabled-focusable.tsx new file mode 100644 index 00000000000..6e6cad44994 --- /dev/null +++ b/apps/design-system/registry/default/example/disabled-focusable.tsx @@ -0,0 +1,26 @@ +'use client' + +import { CirclePause } from 'lucide-react' +import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'ui' + +const UNAVAILABLE_REASON = 'Pausing is unavailable on High Availability projects' + +export default function DisabledFocusable() { + const unavailable = true + + return ( + + + + + {UNAVAILABLE_REASON} + + ) +} diff --git a/apps/design-system/registry/default/example/disabled-unavailable-with-notice.tsx b/apps/design-system/registry/default/example/disabled-unavailable-with-notice.tsx new file mode 100644 index 00000000000..f549173738d --- /dev/null +++ b/apps/design-system/registry/default/example/disabled-unavailable-with-notice.tsx @@ -0,0 +1,50 @@ +'use client' + +import { CirclePause } from 'lucide-react' +import { + Button, + Card, + CardContent, + CardHeader, + CardTitle, + Tooltip, + TooltipContent, + TooltipTrigger, +} from 'ui' +import { Admonition } from 'ui-patterns/Admonition' + +const UNAVAILABLE_REASON = 'Pausing is unavailable on High Availability projects' + +export default function DisabledUnavailableWithNotice() { + const unavailable = true + + return ( + + + Pause project + + + + + + + + {UNAVAILABLE_REASON} + + + + ) +} diff --git a/apps/design-system/registry/default/example/radio-group-card-with-children.tsx b/apps/design-system/registry/default/example/radio-group-card-with-children.tsx index e4f1be820b1..74a9b98ed9a 100644 --- a/apps/design-system/registry/default/example/radio-group-card-with-children.tsx +++ b/apps/design-system/registry/default/example/radio-group-card-with-children.tsx @@ -1,6 +1,8 @@ import SVG from 'react-inlinesvg' import { RadioGroupCard, RadioGroupCardItem } from 'ui' +import { BASE_PATH } from '@/lib/constants' + export default function RadioGroupDemo() { const singleThemes = [ { name: 'Dark', value: 'dark' }, // Classic Supabase dark @@ -13,7 +15,7 @@ export default function RadioGroupDemo() { {singleThemes.map((theme) => ( - + ))} diff --git a/apps/design-system/registry/examples.ts b/apps/design-system/registry/examples.ts index 3f5025f384f..349b1b3d191 100644 --- a/apps/design-system/registry/examples.ts +++ b/apps/design-system/registry/examples.ts @@ -551,6 +551,18 @@ export const examples: Registry = [ registryDependencies: ['dialog', 'button'], files: ['example/dialog-centered-off.tsx'], }, + { + name: 'disabled-focusable', + type: 'components:example', + registryDependencies: ['button', 'tooltip'], + files: ['example/disabled-focusable.tsx'], + }, + { + name: 'disabled-unavailable-with-notice', + type: 'components:example', + registryDependencies: ['admonition', 'button', 'card', 'tooltip'], + files: ['example/disabled-unavailable-with-notice.tsx'], + }, { name: 'drawer-demo', type: 'components:example', diff --git a/apps/studio/components/interfaces/Compute/ComputeList.test.tsx b/apps/studio/components/interfaces/Compute/ComputeList.test.tsx index 436ec0b6fed..f14eb91140f 100644 --- a/apps/studio/components/interfaces/Compute/ComputeList.test.tsx +++ b/apps/studio/components/interfaces/Compute/ComputeList.test.tsx @@ -75,12 +75,12 @@ describe('ComputeList', () => { expect(rowNames()).toHaveLength(10) expect(screen.getByText('Page 1 of 2')).toBeVisible() - expect(screen.getByRole('button', { name: 'Previous page' })).toBeDisabled() + expect(screen.getByRole('button', { name: 'Previous page' })).toBeAriaDisabled() await userEvent.click(screen.getByRole('button', { name: 'Next page' })) expect(rowNames()).toEqual(['instance-10', 'instance-11']) - expect(screen.getByRole('button', { name: 'Next page' })).toBeDisabled() + expect(screen.getByRole('button', { name: 'Next page' })).toBeAriaDisabled() }) it('returns to the first page when a search shrinks the results', async () => { diff --git a/apps/studio/components/interfaces/Database/Backups/BackupItem.test.tsx b/apps/studio/components/interfaces/Database/Backups/BackupItem.test.tsx index 9c7d2d89c52..5c8b1416687 100644 --- a/apps/studio/components/interfaces/Database/Backups/BackupItem.test.tsx +++ b/apps/studio/components/interfaces/Database/Backups/BackupItem.test.tsx @@ -53,10 +53,9 @@ describe('BackupItem', () => { ) const button = screen.getByRole('button', { name: 'Restore' }) - expect(button).toBeDisabled() + expect(button).toBeAriaDisabled() - // Radix opens the tooltip on pointermove; userEvent does not synthesize - // pointer events on disabled buttons + // Radix opens the tooltip on pointermove; focusable disabled buttons support this fireEvent.pointerMove(button) expect( await screen.findAllByText( diff --git a/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx b/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx index bbbec8e9226..b114f5dbb43 100644 --- a/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx +++ b/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx @@ -359,7 +359,7 @@ describe('ExplorerNotebookTab', () => { renderNotebookTab() const runNotebookButton = await screen.findByRole('button', { name: 'Run notebook' }) - expect(runNotebookButton).toBeDisabled() + expect(runNotebookButton).toBeAriaDisabled() }) it('toggles and persists the Intellisense enabled preference from "More options"', async () => { diff --git a/apps/studio/components/interfaces/Explorer/__tests__/QueryTab.test.tsx b/apps/studio/components/interfaces/Explorer/__tests__/QueryTab.test.tsx index cde619b1a7e..fc3e46d707d 100644 --- a/apps/studio/components/interfaces/Explorer/__tests__/QueryTab.test.tsx +++ b/apps/studio/components/interfaces/Explorer/__tests__/QueryTab.test.tsx @@ -282,7 +282,7 @@ describe('QueryTab execution', () => { renderQueryTab() const runButton = await screen.findByRole('button', { name: 'Run' }) - expect(runButton).toBeDisabled() + expect(runButton).toBeAriaDisabled() act(() => releaseReplicas()) await waitFor(() => expect(runButton).toBeEnabled()) diff --git a/apps/studio/components/interfaces/Organization/InvoicesSettings/InvoicesSettings.test.tsx b/apps/studio/components/interfaces/Organization/InvoicesSettings/InvoicesSettings.test.tsx index 9921530f961..63a5ea93ea2 100644 --- a/apps/studio/components/interfaces/Organization/InvoicesSettings/InvoicesSettings.test.tsx +++ b/apps/studio/components/interfaces/Organization/InvoicesSettings/InvoicesSettings.test.tsx @@ -124,7 +124,7 @@ describe('InvoicesSettings', () => { render() - expect(screen.getByRole('button', { name: 'Download invoice' })).toBeDisabled() + expect(screen.getByRole('button', { name: 'Download invoice' })).toBeAriaDisabled() }) it('shows an error when the fetched invoice has no PDF', async () => { diff --git a/apps/studio/components/interfaces/Settings/General/Infrastructure/PauseProjectButton.test.tsx b/apps/studio/components/interfaces/Settings/General/Infrastructure/PauseProjectButton.test.tsx index 0be7102c2fb..9b6dc5a51f3 100644 --- a/apps/studio/components/interfaces/Settings/General/Infrastructure/PauseProjectButton.test.tsx +++ b/apps/studio/components/interfaces/Settings/General/Infrastructure/PauseProjectButton.test.tsx @@ -59,10 +59,9 @@ describe('PauseProjectButton', () => { customRender() const button = screen.getByRole('button', { name: 'Pause project' }) - expect(button).toBeDisabled() + expect(button).toBeAriaDisabled() - // Radix opens the tooltip on pointermove; userEvent does not synthesize - // pointer events on disabled buttons + // Radix opens the tooltip on pointermove; focusable disabled buttons support this fireEvent.pointerMove(button) expect( await screen.findAllByText( diff --git a/apps/studio/components/ui/ButtonTooltip.tsx b/apps/studio/components/ui/ButtonTooltip.tsx index 564867013aa..2789f8dea92 100644 --- a/apps/studio/components/ui/ButtonTooltip.tsx +++ b/apps/studio/components/ui/ButtonTooltip.tsx @@ -1,5 +1,5 @@ import { ComponentProps, ComponentPropsWithoutRef, ElementRef, forwardRef, ReactNode } from 'react' -import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' +import { Button, Tooltip, TooltipContent, TooltipTrigger } from 'ui' export const ButtonTooltip = forwardRef< ElementRef, @@ -10,17 +10,25 @@ export const ButtonTooltip = forwardRef< } } } ->(({ tooltip, className, ...props }, ref) => { +>(({ tooltip, className, disabled, focusableWhenDisabled, ...props }, ref) => { + const { text, ...tooltipContentProps } = tooltip.content + const hasTooltip = text !== undefined + const shouldRemainFocusable = focusableWhenDisabled ?? (disabled === true && hasTooltip) + return ( - - {tooltip.content.text !== undefined && ( - {tooltip.content.text} - )} + {text !== undefined && {text}} ) }) diff --git a/apps/studio/tests/components/interfaces/App/IndirectTaxDeclarationModal.test.tsx b/apps/studio/tests/components/interfaces/App/IndirectTaxDeclarationModal.test.tsx index 1c1b6d6f59c..548d4ab3fac 100644 --- a/apps/studio/tests/components/interfaces/App/IndirectTaxDeclarationModal.test.tsx +++ b/apps/studio/tests/components/interfaces/App/IndirectTaxDeclarationModal.test.tsx @@ -85,7 +85,7 @@ describe('IndirectTaxDeclarationModal', () => { expect(screen.getByRole('dialog')).toBeInTheDocument() const submitButton = screen.getByRole('button', { name: 'Submit declaration' }) - expect(submitButton).toBeDisabled() + expect(submitButton).toBeAriaDisabled() await userEvent.hover(submitButton) expect(await screen.findByRole('tooltip')).toHaveTextContent('Select Yes or No to continue') diff --git a/apps/studio/tests/lib/disabled-matchers.ts b/apps/studio/tests/lib/disabled-matchers.ts new file mode 100644 index 00000000000..8738c772303 --- /dev/null +++ b/apps/studio/tests/lib/disabled-matchers.ts @@ -0,0 +1,25 @@ +import { expect } from 'vitest' + +function isAriaDisabled(element: Element) { + if (!(element instanceof HTMLElement)) return false + + return element.getAttribute('aria-disabled') === 'true' +} + +expect.extend({ + toBeAriaDisabled(received: Element) { + const pass = isAriaDisabled(received) + + return { + pass, + message: () => + pass ? `expected element not to be aria-disabled` : `expected element to be aria-disabled`, + } + }, +}) + +declare module 'vitest' { + interface Matchers = void | Promise, T = unknown> { + toBeAriaDisabled(): R + } +} diff --git a/apps/studio/tests/vitestSetup.ts b/apps/studio/tests/vitestSetup.ts index 40b49a2f1f8..97fc63a2a30 100644 --- a/apps/studio/tests/vitestSetup.ts +++ b/apps/studio/tests/vitestSetup.ts @@ -1,4 +1,5 @@ import '@testing-library/jest-dom/vitest' +import './lib/disabled-matchers' import { cleanup } from '@testing-library/react' import dayjs from 'dayjs' diff --git a/apps/ui-library/components/side-navigation-item.tsx b/apps/ui-library/components/side-navigation-item.tsx index b92af8bfe0a..7808b82b4a8 100644 --- a/apps/ui-library/components/side-navigation-item.tsx +++ b/apps/ui-library/components/side-navigation-item.tsx @@ -91,10 +91,11 @@ const NavigationItem: React.FC = ({ item, onClick, ...props 'items-center justify-between', 'h-6', 'text-sm', - 'text-foreground-lighter px-6', - !isActive && 'hover:bg-surface-100 hover:text-foreground', - isActive && 'bg-surface-200 text-foreground', + 'px-6', 'transition-all', + isActive + ? 'bg-selection text-foreground' + : 'text-foreground-light hover:bg-surface-200 hover:text-foreground', props.className )} > diff --git a/packages/ui/src/components/Button/Button.test.tsx b/packages/ui/src/components/Button/Button.test.tsx index 054b933258d..ac5596a9bc3 100644 --- a/packages/ui/src/components/Button/Button.test.tsx +++ b/packages/ui/src/components/Button/Button.test.tsx @@ -1,6 +1,6 @@ import { fireEvent, render, screen } from '@testing-library/react' import React from 'react' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { Button } from './Button' @@ -52,6 +52,73 @@ describe('#Button', () => { expect(screen.getByText('按钮')).toBeInTheDocument() }) + it('should use native disabled when loading by default', () => { + render() + + const button = screen.getByRole('button') + expect(button).toBeDisabled() + expect(button).not.toHaveAttribute('aria-disabled', 'true') + }) + + it('should remain focusable and ignore events when disabled with focusableWhenDisabled', () => { + const WrapperButton = () => { + const [state, setState] = React.useState('state1') + return ( + + ) + } + + render() + const button = screen.getByRole('button', { name: 'state1' }) + + expect(button).toHaveAttribute('aria-disabled', 'true') + expect(button).not.toBeDisabled() + expect(button).toHaveAttribute('tabIndex', '0') + + fireEvent.click(button) + + expect(screen.getByText('state1')).toBeInTheDocument() + expect(screen.queryByText('state2')).not.toBeInTheDocument() + }) + + it('should ignore child onClick when focusably disabled with asChild', () => { + const childOnClick = vi.fn() + const buttonOnClick = vi.fn() + + render( + + ) + + fireEvent.click(screen.getByRole('link')) + + expect(childOnClick).not.toHaveBeenCalled() + expect(buttonOnClick).not.toHaveBeenCalled() + }) + + it('should not call Button onClick when asChild child calls preventDefault', () => { + const childOnClick = vi.fn((e: React.MouseEvent) => e.preventDefault()) + const buttonOnClick = vi.fn() + + render( + + ) + + fireEvent.click(screen.getByRole('link')) + + expect(childOnClick).toHaveBeenCalled() + expect(buttonOnClick).not.toHaveBeenCalled() + }) + it('should ignore events when disabled', () => { const WrapperButton = () => { const [state, setState] = React.useState('state1') diff --git a/packages/ui/src/components/Button/Button.tsx b/packages/ui/src/components/Button/Button.tsx index 201a793ffe9..c8b838bad56 100644 --- a/packages/ui/src/components/Button/Button.tsx +++ b/packages/ui/src/components/Button/Button.tsx @@ -108,6 +108,9 @@ const buttonVariants = cva( disabled: { true: 'opacity-50 cursor-not-allowed pointer-events-none', }, + focusableWhenDisabled: { + true: 'opacity-50 cursor-not-allowed', + }, rounded: { true: 'rounded-full', }, @@ -184,6 +187,12 @@ export interface ButtonProps iconLeft?: React.ReactNode iconRight?: React.ReactNode rounded?: boolean + /** + * Keeps a disabled button keyboard-focusable by using `aria-disabled` + * instead of native `disabled`. Use this when the control needs a tooltip + * or other explanation. + */ + focusableWhenDisabled?: boolean } const Button = forwardRef( @@ -200,19 +209,23 @@ const Button = forwardRef( iconLeft, type = 'button', rounded, + focusableWhenDisabled: focusableWhenDisabledProp, ...props }, ref ) => { const Comp = asChild ? Slot.Slot : 'button' - const { className, tabIndex } = props + const { className, tabIndex, disabled: disabledProp, onClick, ...rest } = props const showIcon = loading || icon // decrecating 'showIcon' for rightIcon const _iconLeft: React.ReactNode = icon ?? iconLeft + const isLoading = loading === true // if loading, button is disabled - const disabled = loading === true || props.disabled + const disabled = isLoading || disabledProp === true + const focusableWhenDisabled = disabled && focusableWhenDisabledProp === true + const nativeDisabled = disabled && !focusableWhenDisabled - const computedTabIndex = getExplicitTabIndex(tabIndex, disabled) + const computedTabIndex = getExplicitTabIndex(tabIndex, nativeDisabled) const renderIconContainer = (content: ReactNode) => (
@@ -220,26 +233,48 @@ const Button = forwardRef(
) + const handleActivation = (e: React.MouseEvent, childOnClick?: React.MouseEventHandler) => { + if (disabled) { + e.preventDefault() + e.stopPropagation() + return + } + + childOnClick?.(e) + if (!e.defaultPrevented) { + onClick?.(e as React.MouseEvent) + } + } + return ( { - // [Joshen] Prevents redirecting if Button is used with a link-based child element - if (disabled) return e.preventDefault() - else props?.onClick?.(e) - }} + className={cn( + buttonVariants({ + variant, + size, + disabled: nativeDisabled, + focusableWhenDisabled, + block, + rounded, + }), + className + )} + onClick={asChild ? undefined : (e) => handleActivation(e)} > {asChild ? ( - isValidElement<{ children: ReactNode }>(children) ? ( + isValidElement<{ children: ReactNode; onClick?: React.MouseEventHandler }>(children) ? ( cloneElement( children, - undefined, + { + onClick: (e) => handleActivation(e, children.props.onClick), + }, showIcon && (loading ? renderIconContainer(