From 8ea220bb9263968709a1a97a5d1564fd6017e969 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Mon, 21 Sep 2026 18:01:47 +0200 Subject: [PATCH] fix(bindx-ui): preserve keyboard navigation in cell tooltips --- bun.lock | 3 + packages/bindx-ui/package.json | 1 + .../bindx-ui/src/datagrid/ui/label-ui.tsx | 2 +- packages/bindx-ui/src/ui/tooltip.tsx | 87 +++++++++++++++---- packages/example/pages/datagrid.tsx | 2 +- tests/browser/browser.ts | 8 ++ tests/browser/relationTooltipKeyboard.test.ts | 82 +++++++++++++++++ .../dataview/relationTooltipKeyboard.test.tsx | 63 ++++++++++++++ .../dataview/relationTooltipPortal.test.tsx | 4 + 9 files changed, 235 insertions(+), 17 deletions(-) create mode 100644 tests/browser/relationTooltipKeyboard.test.ts create mode 100644 tests/react/dataview/relationTooltipKeyboard.test.tsx diff --git a/bun.lock b/bun.lock index 0ba1396a..75e40e5f 100644 --- a/bun.lock +++ b/bun.lock @@ -140,6 +140,7 @@ "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "lucide-react": "^0.511.0", + "tabbable": "^6.5.0", "tailwind-merge": "^3.3.1", }, "devDependencies": { @@ -856,6 +857,8 @@ "source-map-js": ["source-map-js@1.2.1", "", {}, "sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA=="], + "tabbable": ["tabbable@6.5.0", "", {}, "sha512-wieBHXygIm7OyQOu5hQlkk62/WyCFYGlWg7L6/ZCUZwx0o398Zkn4pVmMyfYhfMG8kGrj/Krt8eIk6UKC6VzwA=="], + "tailwind-merge": ["tailwind-merge@3.7.0", "", {}, "sha512-XPPUyAc+cvspz3lHTcR/QgPfW2A0lv/xQNIjX3HGhLR+Nq2lHaLq5MtTesHn8GUr3W3DguT2KT5x3NVgRtYwmA=="], "tailwindcss": ["tailwindcss@4.3.3", "", {}, "sha512-gOhV3P7ufE62QDGg1zVaTgCR+EtPv92k2nIhVcVKcLmxT1sUBsQGhnZj175j+MqRt4zLF7ic+sCYjfhxMxj7YQ=="], diff --git a/packages/bindx-ui/package.json b/packages/bindx-ui/package.json index 66a22f00..71bb202e 100644 --- a/packages/bindx-ui/package.json +++ b/packages/bindx-ui/package.json @@ -35,6 +35,7 @@ "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "lucide-react": "^0.511.0", + "tabbable": "^6.5.0", "tailwind-merge": "^3.3.1" }, "peerDependencies": { diff --git a/packages/bindx-ui/src/datagrid/ui/label-ui.tsx b/packages/bindx-ui/src/datagrid/ui/label-ui.tsx index ab2a0385..fbaefd55 100644 --- a/packages/bindx-ui/src/datagrid/ui/label-ui.tsx +++ b/packages/bindx-ui/src/datagrid/ui/label-ui.tsx @@ -2,6 +2,6 @@ import { uic } from '../../utils/uic.js' // Focusable so the filter affordance and its tooltip are reachable without a mouse. export const DataGridTooltipLabel = uic('span', { - defaultProps: { tabIndex: 0 }, + defaultProps: { tabIndex: 0, 'aria-keyshortcuts': 'ArrowDown' }, baseClass: 'cursor-pointer underline decoration-dashed decoration-transparent underline-offset-4 transition-colors hover:decoration-gray-400 focus-visible:decoration-gray-800 focus-visible:outline-hidden focus-visible:ring-1 focus-visible:ring-ring rounded-xs', }) diff --git a/packages/bindx-ui/src/ui/tooltip.tsx b/packages/bindx-ui/src/ui/tooltip.tsx index 50251a59..f11e6115 100644 --- a/packages/bindx-ui/src/ui/tooltip.tsx +++ b/packages/bindx-ui/src/ui/tooltip.tsx @@ -11,6 +11,7 @@ */ import * as PopoverPrimitive from '@radix-ui/react-popover' import { forwardRef, useCallback, useEffect, useRef, useState, type ReactNode } from 'react' +import { tabbable, type FocusableElement } from 'tabbable' import { cn } from '../utils/cn.js' export interface TooltipProps { @@ -33,9 +34,10 @@ export const Tooltip = forwardRef(({ const [open, setOpen] = useState(false) const closeTimer = useRef | null>(null) const interaction = useRef({ pointer: false, focus: false }) - // Focus moves into the panel only when the keyboard opened it. A hover must - // leave focus wherever the user put it. - const openedByFocus = useRef(false) + const panelRef = useRef(null) + const focusOrigin = useRef(null) + const enterOnMount = useRef(false) + const restoringFocus = useRef(false) const cancelClose = useCallback((): void => { if (closeTimer.current !== null) { @@ -56,46 +58,101 @@ export const Tooltip = forwardRef(({ const openFor = useCallback((source: 'pointer' | 'focus'): void => { cancelClose() interaction.current[source] = true - openedByFocus.current = source === 'focus' setOpen(true) }, [cancelClose]) + const closePanel = (): void => { + cancelClose() + interaction.current = { pointer: false, focus: false } + enterOnMount.current = false + setOpen(false) + } + + const returnToTrigger = (): void => { + closePanel() + restoringFocus.current = true + focusOrigin.current?.focus({ preventScroll: true }) + restoringFocus.current = false + } + + const focusPanel = (): void => { + const panel = panelRef.current + if (!panel) return + const target = tabbable(panel)[0] ?? panel + target.focus({ preventScroll: true }) + } + useEffect(() => cancelClose, [cancelClose]) return ( { - if (!nextOpen) { - cancelClose() - interaction.current = { pointer: false, focus: false } - } - setOpen(nextOpen) + if (nextOpen) setOpen(true) + else closePanel() }}> {/* Trigger rather than Anchor: Radix excludes the trigger's subtree from its outside-dismissal, so focusing the label does not close the panel. */}
openFor('pointer')} + onPointerEnter={event => { + if (!event.currentTarget.contains(document.activeElement)) { + focusOrigin.current = tabbable(event.currentTarget)[0] ?? event.currentTarget + } + openFor('pointer') + }} onPointerLeave={() => scheduleClose('pointer')} - onFocus={() => openFor('focus')} + onFocus={event => { + focusOrigin.current = event.target + if (!restoringFocus.current) openFor('focus') + }} onBlur={() => scheduleClose('focus')} + onKeyDown={event => { + if (event.key !== 'ArrowDown' || event.defaultPrevented || event.altKey || event.ctrlKey || event.metaKey) return + if (!(event.target instanceof HTMLElement)) return + if (event.target.closest('a,button,input,select,textarea,[contenteditable="true"]')) return + event.preventDefault() + event.stopPropagation() + focusOrigin.current = event.target + if (panelRef.current) focusPanel() + else { + enterOnMount.current = true + openFor('focus') + } + }} > {children}
{ - if (!openedByFocus.current) event.preventDefault() + event.preventDefault() + if (enterOnMount.current) { + enterOnMount.current = false + focusPanel() + } + }} + onCloseAutoFocus={event => event.preventDefault()} + onEscapeKeyDown={event => { + event.preventDefault() + if (panelRef.current?.contains(document.activeElement)) returnToTrigger() + else closePanel() }} - onCloseAutoFocus={event => { - // Restoring focus after hover/blur closure would open the panel again. - if (!interaction.current.focus) event.preventDefault() + onKeyDownCapture={event => { + if (event.key !== 'Tab' || event.defaultPrevented || event.altKey || event.ctrlKey || event.metaKey) return + const stops = tabbable(event.currentTarget) + const edge = event.shiftKey ? stops[0] : stops[stops.length - 1] + if (event.target !== edge && event.target !== event.currentTarget) return + // Bypass Radix's loop; native Tab continues from the original cell. + event.stopPropagation() + returnToTrigger() }} onPointerEnter={() => openFor('pointer')} onPointerLeave={() => scheduleClose('pointer')} diff --git a/packages/example/pages/datagrid.tsx b/packages/example/pages/datagrid.tsx index 05d42730..b5fe382c 100644 --- a/packages/example/pages/datagrid.tsx +++ b/packages/example/pages/datagrid.tsx @@ -59,7 +59,7 @@ export function DataGridPage(): ReactElement { - {author => author.name.value ?? '\u2014'} + {author => {author.name.value ?? '\u2014'}} {tag => tag.name.value ?? ''} diff --git a/tests/browser/browser.ts b/tests/browser/browser.ts index 7289d1b3..68ca5743 100644 --- a/tests/browser/browser.ts +++ b/tests/browser/browser.ts @@ -82,6 +82,7 @@ export interface ElementHandle { attr(name: string): string count(): number click(): void + hover(): void fill(value: string): void select(optionText: string): void } @@ -113,6 +114,9 @@ export function el(selector: string): ElementHandle { exec(`agent-browser click ${quoted}`) Bun.sleepSync(500) }, + hover(): void { + exec(`agent-browser hover ${quoted}`) + }, fill(value: string): void { exec(`agent-browser scrollintoview ${quoted}`) exec(`agent-browser fill ${quoted} ${q(value)}`) @@ -199,6 +203,10 @@ export function evalJs(js: string): string { return exec(`agent-browser eval ${q(js)}`) } +export function press(key: string): void { + exec(`agent-browser press ${q(key)}`) +} + export function screenshot(path?: string): string { const target = path ?? `/tmp/browser-test-${Date.now()}.png` exec(`agent-browser screenshot ${target}`) diff --git a/tests/browser/relationTooltipKeyboard.test.ts b/tests/browser/relationTooltipKeyboard.test.ts new file mode 100644 index 00000000..0c99b703 --- /dev/null +++ b/tests/browser/relationTooltipKeyboard.test.ts @@ -0,0 +1,82 @@ +import { expect, test } from 'bun:test' +import { browserTest, el, evalJs, press, tid, waitFor } from './browser.js' + +const cell = `${tid('datagrid-example')} ${tid('datagrid-row-0')} ${tid('datagrid-cell-author')}` +const label = `${cell} [aria-keyshortcuts="ArrowDown"]` +const panel = '[data-bindx-tooltip-panel]' + +function activeMatches(selector: string): boolean { + return evalJs(`document.activeElement?.matches(${JSON.stringify(selector)})`) === 'true' +} + +function focusLabel(): void { + evalJs(`document.querySelector(${JSON.stringify(label)}).focus()`) + waitFor(() => el(panel).exists) + expect(activeMatches(label)).toBe(true) +} + +browserTest('relation tooltip keyboard navigation', () => { + test('keeps cell links in native tab order and follows them with Enter', () => { + waitFor(() => el(label).exists) + focusLabel() + press('Tab') + expect(activeMatches(`${cell} a`)).toBe(true) + press('Shift+Tab') + expect(activeMatches(label)).toBe(true) + press('Tab') + press('Enter') + waitFor(() => evalJs('location.hash') === '"#entity-lists"') + evalJs('location.hash = "datagrid"') + waitFor(() => el(label).exists) + }) + + test('ArrowDown enters actions; Escape restores focus without reopening', () => { + focusLabel() + press('ArrowDown') + expect(activeMatches(`${panel} button:first-child`)).toBe(true) + press('Escape') + waitFor(() => !el(panel).exists) + expect(activeMatches(label)).toBe(true) + press('ArrowDown') + waitFor(() => activeMatches(`${panel} button:first-child`)) + press('Escape') + waitFor(() => !el(panel).exists) + press('Tab') + expect(activeMatches(`${cell} a`)).toBe(true) + }) + + test('Tab traverses both actions then continues at the cell link', () => { + focusLabel() + press('ArrowDown') + press('Tab') + expect(activeMatches(`${panel} button:last-child`)).toBe(true) + press('Tab') + expect(activeMatches(`${cell} a`)).toBe(true) + press('Tab') + expect(activeMatches(`${panel} *`)).toBe(false) + expect(activeMatches(`${cell} *`)).toBe(false) + expect(activeMatches('body')).toBe(false) + }) + + test('Shift+Tab at the first action leaves the panel backwards', () => { + focusLabel() + press('ArrowDown') + press('Shift+Tab') + expect(activeMatches(`${panel} *`)).toBe(false) + expect(activeMatches('body')).toBe(false) + press('Tab') + expect(activeMatches(label)).toBe(true) + }) + + test('Escape restores the cell after entering a hover-opened panel', () => { + press('Escape') + waitFor(() => !el(panel).exists) + evalJs('document.activeElement.blur()') + el(label).hover() + waitFor(() => el(panel).exists) + evalJs(`document.querySelector('${panel} button').focus()`) + press('Escape') + waitFor(() => !el(panel).exists) + expect(activeMatches(label)).toBe(true) + }) +}, 'datagrid') diff --git a/tests/react/dataview/relationTooltipKeyboard.test.tsx b/tests/react/dataview/relationTooltipKeyboard.test.tsx new file mode 100644 index 00000000..8732bffd --- /dev/null +++ b/tests/react/dataview/relationTooltipKeyboard.test.tsx @@ -0,0 +1,63 @@ +import '../../setup' +import { afterEach, expect, test } from 'bun:test' +import { act, cleanup, fireEvent, render, waitFor } from '@testing-library/react' +import React from 'react' +import { Tooltip, DataGridTooltipLabel } from '@contember/bindx-ui' + +afterEach(async () => { + await act(async () => { + cleanup() + await new Promise(resolve => setTimeout(resolve, 0)) + }) +}) + +function renderCell(): ReturnType { + return render( + }> + Author + , + ) +} + +test('focus reveals actions without stealing focus from the label or its link', async () => { + const { getByTestId, getByRole } = renderCell() + const label = getByTestId('label') + act(() => label.focus()) + await waitFor(() => expect(getByRole('button', { name: 'Filter' })).not.toBeNull()) + expect(document.activeElement === label).toBe(true) + const link = getByRole('link') + act(() => link.focus()) + expect(document.activeElement === link).toBe(true) + fireEvent.keyDown(link, { key: 'ArrowDown' }) + expect(document.activeElement === link).toBe(true) +}) + +test('Escape returns to the trigger without reopening; ArrowDown can enter again', async () => { + const { getByTestId, getByRole } = renderCell() + const label = getByTestId('label') + act(() => label.focus()) + await waitFor(() => expect(getByRole('button', { name: 'Filter' })).not.toBeNull()) + act(() => getByRole('button', { name: 'Filter' }).focus()) + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')).toBeNull()) + expect(document.activeElement === label).toBe(true) + fireEvent.keyDown(label, { key: 'ArrowDown' }) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')?.contains(document.activeElement)).toBe(true)) +}) + +test('Escape on a preview keeps focus on the link and outside dismissal preserves the new focus', async () => { + const { getByRole } = renderCell() + const { getByText } = render() + const link = getByRole('link') + act(() => link.focus()) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')).not.toBeNull()) + fireEvent.keyDown(link, { key: 'Escape' }) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')).toBeNull()) + expect(document.activeElement === link).toBe(true) + act(() => getByText('Outside').focus()) + act(() => link.focus()) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')).not.toBeNull()) + act(() => getByText('Outside').focus()) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel]')).toBeNull()) + expect(document.activeElement === getByText('Outside')).toBe(true) +}) diff --git a/tests/react/dataview/relationTooltipPortal.test.tsx b/tests/react/dataview/relationTooltipPortal.test.tsx index 700d588b..d9d8f1bf 100644 --- a/tests/react/dataview/relationTooltipPortal.test.tsx +++ b/tests/react/dataview/relationTooltipPortal.test.tsx @@ -81,6 +81,8 @@ describe('relation column filter affordance', () => { }) const label = getByTestId(container, 'datagrid-cell-author').querySelector('[tabindex="0"]')! act(() => label.focus()) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel] button')).not.toBeNull()) + act(() => document.querySelector('[data-bindx-tooltip-panel] button')!.focus()) await waitFor(() => expect(document.activeElement?.textContent).toBe('Filter')) const panel = document.querySelector('[data-bindx-tooltip-panel]')! @@ -105,6 +107,8 @@ describe('relation column filter affordance', () => { }) const label = getByTestId(container, 'datagrid-cell-author').querySelector('[tabindex="0"]')! act(() => label.focus()) + await waitFor(() => expect(document.querySelector('[data-bindx-tooltip-panel] button')).not.toBeNull()) + act(() => document.querySelector('[data-bindx-tooltip-panel] button')!.focus()) await waitFor(() => expect(document.activeElement?.textContent).toBe('Filter')) const panel = document.querySelector('[data-bindx-tooltip-panel]')!