From 37296128e07a1810c00a5a6130074f3fc3269c79 Mon Sep 17 00:00:00 2001
From: Danny White <3104761+dnywh@users.noreply.github.com>
Date: Mon, 20 Jul 2026 18:00:24 -0400
Subject: [PATCH] fix(studio): untangle nested buttons in editor tabs (#48101)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## What kind of change does this PR introduce?

A11y markup + keyboard close for editor tabs (Table Editor open tables /
SQL Editor snippets).

## What is the current behavior?

Editor tabs nest interactive elements (already in prod):

1. Sortable shell spreads dnd-kit `attributes` → `div role="button"
tabindex="0"`
2. Inner `TabsTrigger` → real `<button role="tab">`
3. Close control → `role="button"` nested inside the tab button

Close was hover/pointer-only. There was no clear keyboard path.

## What is the new behavior?

**Markup**

- Sortable shell is a plain `div` (no dnd-kit `attributes` / no
`role="button"`). Safe because tab reorder only uses `PointerSensor`.
- Close is a real `<button type="button">` **sibling** of `TabsTrigger`.
- Same for the non-draggable “New” tab.

**Keyboard close**

- ←/→ still move between tabs (Radix roving tabindex — Tab key does not
walk every tab).
- **Delete** or **Backspace** on a focused tab closes it.
- The **active** tab’s close button is in the tab order
(`tabIndex={0}`); Tab from the active tab reaches ×, then Enter/Space
closes. Inactive closes stay `tabIndex={-1}`.
- Close shows on hover, focus-within, and focus-visible (with focus
ring).

## Test plan

### Markup
- [ ] Inspect DOM: no `role="button"` wrapper around `role="tab"`; close
is not nested inside the tab button

### Mouse
- [ ] Hover → ×; click × closes
- [ ] Drag reorder still works
- [ ] Middle-click / double-click pin / “New” tab close still work
- [ ] Context menu → Close still works

### Keyboard
- [ ] Focus the active tab (Tab into the strip, or click then Tab)
- [ ] ←/→ moves across tabs
- [ ] Delete or Backspace closes the focused tab
- [ ] From the active tab, Tab once focuses × (visible + ring);
Enter/Space closes
- [ ] Delete while focus is in the table grid / SQL editor (not on a
tab) does **not** close tabs

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Added dedicated close buttons for tabs, with visibility on hover or
focus.
* Tabs can now be closed using the Delete or Backspace keys when
focused.

* **Bug Fixes**
* Improved tab selection and drag interactions when clicking or pressing
tab close controls.
* Prevented closing a tab from unintentionally activating or dragging
it.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---
 .../components/layouts/Tabs/SortableTab.tsx   | 172 ++++++++++--------
 apps/studio/components/layouts/Tabs/Tabs.tsx  |  59 +++---
 2 files changed, 138 insertions(+), 93 deletions(-)

diff --git a/apps/studio/components/layouts/Tabs/SortableTab.tsx b/apps/studio/components/layouts/Tabs/SortableTab.tsx
index 4e9a99d2095..23240722b91 100644
--- a/apps/studio/components/layouts/Tabs/SortableTab.tsx
+++ b/apps/studio/components/layouts/Tabs/SortableTab.tsx
@@ -2,7 +2,7 @@ import { useSortable } from '@dnd-kit/sortable'
 import { CSS } from '@dnd-kit/utilities'
 import { AnimatePresence, motion } from 'framer-motion'
 import { X } from 'lucide-react'
-import { useMemo } from 'react'
+import { useMemo, type KeyboardEvent } from 'react'
 import { cn, TabsTrigger } from 'ui'
 
 import { useEditorType } from '../editors/EditorsLayout.hooks'
@@ -17,6 +17,14 @@ import { useTabsStateSnapshot, type Tab } from '@/state/tabs'
  * - Dynamic schema name display
  * - Tab label animations
  * - Close button interactions
+ *
+ * Markup: sortable shell (plain div) → TabsTrigger + close as siblings.
+ * dnd-kit `attributes` are intentionally not spread on the shell — they inject
+ * `role="button"` / `tabIndex={0}`, which nested a second button around the tab.
+ * Only PointerSensor is used for reorder, so those attributes are not required.
+ *
+ * Keyboard: ←/→ move between tabs (Radix). Delete/Backspace on a focused tab
+ * closes it. The active tab's close button is in the tab order.
  */
 export const SortableTab = ({
   tab,
@@ -37,7 +45,7 @@ export const SortableTab = ({
   void tabs.handlerRegistrationVersion
   const StatusIndicator = tabs.getTabStatusIndicator(tab.type)
   const { selectedSchema: currentSchema } = useQuerySchemaState()
-  const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({
+  const { setNodeRef, listeners, transform, transition, isDragging } = useSortable({
     id: tab.id,
   })
 
@@ -54,89 +62,109 @@ export const SortableTab = ({
     return openTabs.some((t) => editor === 'table' && t.metadata?.schema !== currentSchema)
   }, [openTabs, currentSchema, editor])
 
-  // Create a motion version of TabsTrigger while preserving all functionality
-  // const MotionTabsTrigger = motion(TabsTrigger)
+  const isActive = tabs.activeTab === tab.id
+
+  const closeTabFromKeyboard = (event: KeyboardEvent) => {
+    if (event.key !== 'Delete' && event.key !== 'Backspace') return
+    event.preventDefault()
+    event.stopPropagation()
+    onClose(tab.id)
+  }
 
   return (
     <motion.div
       ref={setNodeRef}
       style={style}
-      {...attributes}
       layoutId={tab.id}
       transition={{ duration: 0.045 }}
       animate={{ opacity: isDragging ? 0 : 1 }}
       className={cn('flex items-center h-(--header-height) first-of-type:border-l')}
     >
-      <TabsTrigger
-        value={tab.id}
-        onAuxClick={(e) => {
-          // Middle click closes tab
-          if (e.button === 1) {
-            e.preventDefault()
-            onClose(tab.id)
-          }
-        }}
-        onDoubleClick={() => tabs.makeTabPermanent(tab.id)}
-        className={cn(
-          'flex items-center gap-2 pl-3 pr-2.5 text-xs',
-          'bg-dash-sidebar/50 dark:bg-surface-100/50',
-          'data-[state=active]:bg-dash-sidebar dark:data-[state=active]:bg-surface-100',
-          'border-b border-default',
-          'data-[state=active]:border-b-background-dash-sidebar dark:data-[state=active]:border-b-background-surface-100',
-          'relative group h-full',
-          'hover:bg-surface-300 dark:hover:bg-surface-100',
-          tab.isPreview && 'italic font-light' // Optional: style preview tabs differently
-        )}
-        {...listeners}
-      >
-        <EntityTypeIcon type={tab.type} />
-        <div className="flex items-center gap-0">
-          <AnimatePresence mode="popLayout" initial>
-            {shouldShowSchema && (
-              <motion.span
-                initial={{ opacity: 0, width: 0 }}
-                animate={{ opacity: 1, width: 'auto' }}
-                exit={{ opacity: 0, width: 0 }}
-                transition={{ duration: 0.15 }}
-                className="text-foreground-muted group-data-[state=active]:text-foreground-lighter"
-              >
-                {tab?.metadata?.schema}.
-              </motion.span>
-            )}
-          </AnimatePresence>
-          <span>{tab.label || 'Untitled'}</span>
-        </div>
-        {/* VS Code-style slot: the type's status indicator (e.g. an unsaved dot)
-            shows at rest and swaps to the close button on hover. */}
-        <div className="relative ml-1 flex size-5 items-center justify-center">
-          {StatusIndicator && (
-            <span className="absolute inset-0 flex items-center justify-center group-hover:opacity-0">
-              <StatusIndicator tab={tab} />
-            </span>
-          )}
-          <span
-            role="button"
-            aria-label="Close tab"
-            onClick={(e) => {
+      <div className="group/tab relative flex h-full min-w-0 items-center">
+        <TabsTrigger
+          value={tab.id}
+          onAuxClick={(e) => {
+            // Middle click closes tab
+            if (e.button === 1) {
               e.preventDefault()
-              e.stopPropagation()
-            }}
-            className="absolute inset-0 flex items-center justify-center opacity-0 group-hover:opacity-100 hover:bg-200 rounded-xs cursor-pointer"
-            onMouseDown={(e) => {
-              e.preventDefault()
-              e.stopPropagation()
-            }}
-            onPointerDown={(e) => {
-              e.preventDefault()
-              e.stopPropagation()
               onClose(tab.id)
-            }}
+            }
+          }}
+          onDoubleClick={() => tabs.makeTabPermanent(tab.id)}
+          onKeyDown={closeTabFromKeyboard}
+          className={cn(
+            'flex items-center gap-2 pl-3 pr-2.5 text-xs',
+            'bg-dash-sidebar/50 dark:bg-surface-100/50',
+            'data-[state=active]:bg-dash-sidebar dark:data-[state=active]:bg-surface-100',
+            'border-b border-default',
+            'data-[state=active]:border-b-background-dash-sidebar dark:data-[state=active]:border-b-background-surface-100',
+            'relative group h-full',
+            'hover:bg-surface-300 dark:hover:bg-surface-100',
+            tab.isPreview && 'italic font-light' // Optional: style preview tabs differently
+          )}
+          {...listeners}
+        >
+          <EntityTypeIcon type={tab.type} />
+          <div className="flex items-center gap-0">
+            <AnimatePresence mode="popLayout" initial>
+              {shouldShowSchema && (
+                <motion.span
+                  initial={{ opacity: 0, width: 0 }}
+                  animate={{ opacity: 1, width: 'auto' }}
+                  exit={{ opacity: 0, width: 0 }}
+                  transition={{ duration: 0.15 }}
+                  className="text-foreground-muted group-data-[state=active]:text-foreground-lighter"
+                >
+                  {tab?.metadata?.schema}.
+                </motion.span>
+              )}
+            </AnimatePresence>
+            <span>{tab.label || 'Untitled'}</span>
+          </div>
+          {/* Reserve status/close slot width; close is a sibling overlay, not nested. */}
+          <div
+            className="relative ml-1 flex size-5 shrink-0 items-center justify-center"
+            aria-hidden
           >
-            <X size={12} className="text-foreground-light" />
-          </span>
-        </div>
-        <div className="absolute w-full top-0 left-0 right-0 h-px bg-foreground opacity-0 group-data-[state=active]:opacity-100" />
-      </TabsTrigger>
+            {StatusIndicator && (
+              <span className="absolute inset-0 flex items-center justify-center group-hover/tab:opacity-0 group-focus-within/tab:opacity-0">
+                <StatusIndicator tab={tab} />
+              </span>
+            )}
+          </div>
+          <div className="absolute w-full top-0 left-0 right-0 h-px bg-foreground opacity-0 group-data-[state=active]:opacity-100" />
+        </TabsTrigger>
+        {/* Sibling of TabsTrigger — not nested inside the tab button.
+            Only the active tab's close is in the tab order (roving tabs). Delete/Backspace
+            on the focused tab also closes. */}
+        <button
+          type="button"
+          tabIndex={isActive ? 0 : -1}
+          aria-label="Close tab"
+          onClick={(e) => {
+            e.preventDefault()
+            e.stopPropagation()
+            onClose(tab.id)
+          }}
+          onMouseDown={(e) => {
+            e.preventDefault()
+            e.stopPropagation()
+          }}
+          onPointerDown={(e) => {
+            e.preventDefault()
+            e.stopPropagation()
+          }}
+          className={cn(
+            'absolute top-1/2 right-2.5 z-10 -translate-y-1/2',
+            'flex size-5 items-center justify-center rounded-xs',
+            'opacity-0 group-hover/tab:opacity-100 group-focus-within/tab:opacity-100 focus-visible:opacity-100',
+            'hover:bg-200 focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background',
+            'cursor-pointer'
+          )}
+        >
+          <X size={12} className="text-foreground-light" />
+        </button>
+      </div>
       {index < openTabs.length && (
         <div role="separator" className="h-full w-px bg-border" key={`separator-${tab.id}`} />
       )}
diff --git a/apps/studio/components/layouts/Tabs/Tabs.tsx b/apps/studio/components/layouts/Tabs/Tabs.tsx
index c1959e98717..72b06e913ac 100644
--- a/apps/studio/components/layouts/Tabs/Tabs.tsx
+++ b/apps/studio/components/layouts/Tabs/Tabs.tsx
@@ -214,27 +214,40 @@ export const EditorTabs = () => {
 
             {/* Non-draggable new tab */}
             {hasNewTab && (
-              <TabsTrigger
-                value="new"
-                className={cn(
-                  'flex items-center gap-2 px-3 text-xs',
-                  'bg-dash-sidebar/50 dark:bg-surface-100/50',
-                  'data-[state=active]:bg-dash-sidebar dark:data-[state=active]:bg-surface-100',
-                  'relative group h-full border-t-2 border-b-0!',
-                  'hover:bg-surface-300 dark:hover:bg-surface-100'
-                )}
-              >
-                <Plus size={16} strokeWidth={1.5} className={'text-foreground-lighter'} />
-                <div className="flex items-center gap-0">
-                  <span>New</span>
-                </div>
-                <span
-                  role="button"
+              <div className="group/new-tab relative flex h-full items-center">
+                <TabsTrigger
+                  value="new"
+                  onKeyDown={(e) => {
+                    if (e.key !== 'Delete' && e.key !== 'Backspace') return
+                    e.preventDefault()
+                    e.stopPropagation()
+                    handleClose('new')
+                  }}
+                  className={cn(
+                    'flex items-center gap-2 px-3 text-xs',
+                    'bg-dash-sidebar/50 dark:bg-surface-100/50',
+                    'data-[state=active]:bg-dash-sidebar dark:data-[state=active]:bg-surface-100',
+                    'relative group h-full border-t-2 border-b-0!',
+                    'hover:bg-surface-300 dark:hover:bg-surface-100'
+                  )}
+                >
+                  <Plus size={16} strokeWidth={1.5} className={'text-foreground-lighter'} />
+                  <div className="flex items-center gap-0">
+                    <span>New</span>
+                  </div>
+                  {/* Reserve close-icon width; close is a sibling overlay. */}
+                  <span className="ml-1 inline-flex size-3 shrink-0" aria-hidden />
+                  <div className="absolute w-full -bottom-px left-0 right-0 h-px bg-dash-sidebar dark:bg-surface-100 opacity-0 group-data-[state=active]:opacity-100" />
+                </TabsTrigger>
+                <button
+                  type="button"
+                  tabIndex={0}
+                  aria-label="Close new tab"
                   onClick={(e) => {
                     e.preventDefault()
                     e.stopPropagation()
+                    handleClose('new')
                   }}
-                  className="ml-1 opacity-0 group-hover:opacity-100 hover:bg-200 rounded-xs cursor-pointer"
                   onMouseDown={(e) => {
                     e.preventDefault()
                     e.stopPropagation()
@@ -242,13 +255,17 @@ export const EditorTabs = () => {
                   onPointerDown={(e) => {
                     e.preventDefault()
                     e.stopPropagation()
-                    handleClose('new')
                   }}
+                  className={cn(
+                    'absolute top-1/2 right-3 z-10 flex -translate-y-1/2 items-center justify-center rounded-xs',
+                    'opacity-0 group-hover/new-tab:opacity-100 group-focus-within/new-tab:opacity-100 focus-visible:opacity-100',
+                    'hover:bg-200 focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background',
+                    'cursor-pointer'
+                  )}
                 >
                   <X size={12} className="text-foreground-light" />
-                </span>{' '}
-                <div className="absolute w-full -bottom-px left-0 right-0 h-px bg-dash-sidebar dark:bg-surface-100 opacity-0 group-data-[state=active]:opacity-100" />
-              </TabsTrigger>
+                </button>
+              </div>
             )}
 
             <AnimatePresence initial={false}>