fix(studio): keyboard focus on button (#40458)

* add necessary prop to main button component

* remove redundant props

* docs

* docs improvements

* typo

* tabbable sidebar items

* aria-label and aria-hidden

* update docs

* clarification

* fix link

---------

Co-authored-by: Ali Waseem <waseema393@gmail.com>

apps/design-system/content/docs/accessibility.mdx

@@ -21,18 +21,31 @@ About to push some code? At a minimum, check your work against this list:
 
 ## Focus management
 
-All interactive page elements should be reachable by keyboard. They should also provide visual feedback upon selection via a `focus-visible` state. We use consistent focus styles such as `inset-focus` so users recognize this state instantly.
+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.
 
-```jsx
-<TableCell>
-  <p>{name}</p>
-  <button
-    className={cn('absolute inset-0', 'inset-focus')}
-    onClick={(event) => handleBucketNavigation(name, event)}
-  >
-    <span className="sr-only">Go to bucket details</span>
-  </button>
-</TableCell>
+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.
+
+Interactive page elements should also provide visual feedback upon selection via a `focus-visible` state. We use consistent focus styles such as `inset-focus` so users recognize this state instantly.
+
+[Button](../components/button) has all of the above built-in. Bespoke interactive elements however, such as the below clickable [TableRow](../components/table), require these props to be added manually:
+
+```tsx showLineNumbers {3, 5-11}
+<TableRow
+  key={id}
+  className="relative cursor-pointer h-16 inset-focus"
+  onClick={(event) => handleBucketNavigation(name, event)}
+  onKeyDown={(event) => {
+    if (event.key === 'Enter' || event.key === ' ') {
+      event.preventDefault()
+      handleBucketNavigation(name, event)
+    }
+  }}
+  tabIndex={0}
+>
+  <TableCell>
+    <p>{name}</p>
+  </TableCell>
+</TableRow>
 ```
 
 Consider also affordances like `ctrl` and `meta` key support for opening in a new tab. Anything that you can do with a mouse input should be replicable by keyboard.
@@ -51,13 +64,45 @@ Some keyboard-navigable content may be contain hundreds or thousands of items. H
 - Pagination or virtualization
 - “Jump to” shortcuts to skip ahead
 
-## Screen reader support
+## Screen readers
 
-Textual elements are supported out-of-the-box by screen readers. Imagery of course should be described by `alt` tags.
+Textual elements are supported out-of-the-box by screen readers.
 
-Less obvious however are scaffolding elements that only makes sense visually, when paired with other content. For example: a table column for actions may not have a visual _Actions_ label because its purpose is obvious to a sighted person. For everyone else’s sake, this column should be titled with `sr-only` text:
+### Imagery
 
-```jsx
+Images should have their contents described with an `alt` attribute. Write an objective description of the content rather than its context. For example:
+
+```tsx showLineNumbers {2}
+// Correct: painting a picture with words
+<img src="beagle.png" alt="A tricolor beagle galloping through a grassy field, ears in the air" />
+
+// Incorrect: Unhelpful context
+<img src="beagle.png" alt="Our logo" />
+```
+
+Icons and other visual elements that aren’t strictly images should use the `aria-label` attribute. For example:
+
+```tsx showLineNumbers {2}
+<BucketTableCell>
+  <BucketIcon aria-label="bucket icon" size={16} />
+</BucketTableCell>
+```
+
+Visual elements that are _purely_ visual aids may be removed from the accessibility tree via the `aria-hidden` attribute. For example:
+
+```tsx showLineNumbers {2}
+<BucketTableCell>
+  <ChevronRight aria-hidden={true} size={14} />
+</BucketTableCell>
+```
+
+Never use `aria-hidden={true}` on focusable elements, since these are critical pieces of functionality.
+
+### Scaffolding
+
+Some scaffolding elements only make sense visually, in the context of surrounding visual content. For example: a table column for actions may not have a visual _Actions_ label because its purpose is obvious (by nearby contents) to a sighted person. For everyone else’s sake, this column should be titled with `sr-only` text:
+
+```tsx showLineNumbers {2}
 <TableHead>
   <span className="sr-only">Actions</span>
 </TableHead>

apps/design-system/content/docs/components/button.mdx

@@ -122,3 +122,13 @@ Displaying only an Icon in a button.
 Supports slot behavior with `asChild` prop.
 
 <ComponentPreview name="button-as-child" />
+
+## Accessibility
+
+[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)
+- You can still override with an explicit `tabIndex` prop when needed
+
+You therefore don't need to manually set `tabIndex`, as Button handles it automatically based on its `disabled` state.

apps/studio/components/interfaces/Storage/AnalyticsBuckets/CreateAnalyticsBucketModal.tsx

@@ -244,7 +244,6 @@ export const CreateAnalyticsBucketModal = ({
           className={buttonClassName}
           icon={<Plus size={14} />}
           disabled={isDisabled}
-          tabIndex={isDisabled ? -1 : 0}
           style={{ justifyContent: 'start' }}
           onClick={() => setVisible(true)}
           tooltip={{

apps/studio/components/interfaces/Storage/FilesBuckets/index.tsx

@@ -104,7 +104,7 @@ export const FilesBuckets = () => {
                   />
                   <DropdownMenu>
                     <DropdownMenuTrigger asChild>
-                      <Button tabIndex={0} type="default" icon={<ArrowDownNarrowWide />}>
+                      <Button type="default" icon={<ArrowDownNarrowWide />}>
                         Sorted by {snap.sortBucket === 'alphabetical' ? 'name' : 'created at'}
                       </Button>
                     </DropdownMenuTrigger>

apps/studio/components/interfaces/Storage/VectorBuckets/CreateVectorBucketDialog.tsx

@@ -178,7 +178,6 @@ export const CreateVectorBucketDialog = () => {
           className="w-fit"
           icon={<Plus size={14} />}
           disabled={!canCreateBuckets}
-          tabIndex={!canCreateBuckets ? -1 : 0}
           onClick={() => setVisible(true)}
           tooltip={{
             content: {

packages/ui/src/components/Button/Button.tsx

@@ -239,13 +239,19 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
     ref
   ) => {
     const Comp = asChild ? Slot : 'button'
-    const { className } = props
+    const { className, tabIndex } = props
     const showIcon = loading || icon
     // decrecating 'showIcon' for rightIcon
     const _iconLeft: React.ReactNode = icon ?? iconLeft
     // if loading, button is disabled
     const disabled = loading === true || props.disabled
 
+    // Set default tabIndex for proper Safari focus handling
+    // - Explicit tabIndex prop takes precedence
+    // - If disabled, default to -1 (unless explicitly set)
+    // - Otherwise, default to 0 for keyboard accessibility
+    const computedTabIndex = tabIndex !== undefined ? tabIndex : disabled ? -1 : 0
+
     return (
       <Comp
         ref={ref}
@@ -253,6 +259,7 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
         type={htmlType}
         {...props}
         disabled={disabled}
+        tabIndex={computedTabIndex}
         className={cn(buttonVariants({ type, size, disabled, block, rounded }), className)}
         onClick={(e) => {
           // [Joshen] Prevents redirecting if Button is used with a link-based child element

packages/ui/src/components/shadcn/ui/sidebar.tsx

@@ -553,6 +553,13 @@ const SidebarMenuButton = React.forwardRef<
   ) => {
     const Comp = asChild ? Slot : 'button'
     const { isMobile, state } = useSidebar()
+    const { disabled, tabIndex } = props
+
+    // Set default tabIndex for proper Safari focus handling
+    // - Explicit tabIndex prop takes precedence
+    // - If disabled, default to -1 (unless explicitly set)
+    // - Otherwise, default to 0 for keyboard accessibility
+    const computedTabIndex = tabIndex !== undefined ? tabIndex : disabled ? -1 : 0
 
     const button = (
       <Comp
@@ -561,6 +568,7 @@ const SidebarMenuButton = React.forwardRef<
         data-size={size}
         data-active={isActive}
         data-has-icon={hasIcon}
+        tabIndex={computedTabIndex}
         className={cn(sidebarMenuButtonVariants({ variant, size, hasIcon }), className)}
         {...props}
       />