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(