From d904fb17000326a1467906b7a0f5fd59edcff18e Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 07:00:44 +0200 Subject: [PATCH 1/9] Add a single-Tab-stop focus mode to Toolbar and ActionMenubar Components-owned tools read their tab index from a toolbar context, so React stays the only writer of the attribute. The toolbar keeps the active tool in state and moves it when the tool becomes unavailable. Consumer controls, widgets, and tools with an explicit tab index keep their own Tab stop. Refs #353. --- Documentation/Common/action-menubar.md | 4 +- Documentation/Toolbar/index.md | 3 +- Source/Common/ActionMenubar.stories.tsx | 24 ++++ Source/Common/ActionMenubar.tsx | 43 +++--- Source/Common/ToolbarFocusMode.ts | 8 +- Source/Common/ToolbarRovingContext.ts | 53 ++++++++ Source/Common/useToolbarKeyboardNavigation.ts | 56 +++++++- Source/Toolbar/Toolbar.tsx | 7 +- Source/Toolbar/ToolbarButton.tsx | 4 + Source/Toolbar/ToolbarFanOutItem.tsx | 9 +- Source/Toolbar/ToolbarFolder.tsx | 9 +- .../when_using_a_single_tab_stop.tsx | 126 ++++++++++++++++++ Storybook/scripts/storybook-inventory.json | 2 + 13 files changed, 321 insertions(+), 27 deletions(-) create mode 100644 Source/Common/ToolbarRovingContext.ts create mode 100644 Source/Toolbar/for_Toolbar/when_using_a_single_tab_stop.tsx diff --git a/Documentation/Common/action-menubar.md b/Documentation/Common/action-menubar.md index fe7ec835..3f7d4cda 100644 --- a/Documentation/Common/action-menubar.md +++ b/Documentation/Common/action-menubar.md @@ -29,7 +29,7 @@ The root is a `div` with `role='toolbar'` and `data-cratis-part='root'`. Pass `a Each action renders as a native ghost `Button`. By default, `focusMode={ToolbarFocusMode.Arrows}` moves between actions with Left/Right (reversed in RTL), and Home/End move to the first/last available action. Navigation follows DOM order, skips disabled, hidden, and inert actions, and does not wrap. Every action remains its own Tab stop, preserving Tab and Shift+Tab behavior. -Import `ToolbarFocusMode` from `@cratis/components/Common` and set `focusMode={ToolbarFocusMode.None}` to disable toolbar key handling while retaining native Tab stops. Widgets in templates keep their own keys. Child handlers and `pt.root.onKeyDown` can prevent navigation. A single-Tab-stop mode is planned ([#353](https://github.com/Cratis/Components/issues/353)). +Import `ToolbarFocusMode` from `@cratis/components/Common`. Set `focusMode={ToolbarFocusMode.SingleTabStop}` to give the menubar's own actions one Tab stop that follows the arrow keys and returns to the last focused action; template content and actions with an explicit `pt.root.tabIndex` keep their own Tab stop. Set `focusMode={ToolbarFocusMode.None}` to disable toolbar key handling while retaining native Tab stops. Widgets in templates keep their own keys. Child handlers and `pt.root.onKeyDown` can prevent navigation. `SingleTabStop` becomes the default in the next major release ([#353](https://github.com/Cratis/Components/issues/353)). An item's visible `label` is also its accessible name, and `ActionMenuItem` has no separate `aria-label`, so give every item a `label` or use `template` for an icon-only action that names itself. @@ -52,7 +52,7 @@ When `template` is present, `ActionMenubar` renders its result directly instead | Prop | Type | Required | Purpose | | ------------ | ------------------ | -------- | ---------------------------------------------------------- | | `model` | `ActionMenuItem[]` | Yes | Actions rendered from left to right. | -| `focusMode` | `ToolbarFocusMode` | No | Keyboard focus mode; defaults to `Arrows`. | +| `focusMode` | `ToolbarFocusMode` | No | `Arrows` (default), `SingleTabStop`, or `None`. | | `className` | `string` | No | Extra class name for the toolbar root. | | `aria-label` | `string` | No | Accessible name for the toolbar. | | `pt` | `ButtonParts` | No | Part attributes applied to every non-template action button. | diff --git a/Documentation/Toolbar/index.md b/Documentation/Toolbar/index.md index 2eaaf608..a23a530a 100644 --- a/Documentation/Toolbar/index.md +++ b/Documentation/Toolbar/index.md @@ -21,7 +21,8 @@ import { Toolbar, ToolbarButton } from '@cratis/components/Toolbar'; - The root renders `role='toolbar'`, `aria-orientation`, and an accessible name. The name falls back to the provider's `messages.toolbar.label`, then `Tools`; pass `aria-label` or `aria-labelledby` to name each toolbar. - `focusMode` defaults to `ToolbarFocusMode.Arrows`: Up/Down move between tools in the default vertical orientation; a horizontal toolbar uses Left/Right (reversed in RTL). Home/End move to the first/last available tool in DOM order. Navigation skips disabled, hidden, and inert tools and stops at either end; it does not wrap. Each tool remains its own Tab stop, so Tab and Shift+Tab work as before. -- Set `focusMode={ToolbarFocusMode.None}` to keep native Tab behavior without toolbar key handling. Import `ToolbarFocusMode` from `@cratis/components/Toolbar`. A single-Tab-stop mode is planned ([#353](https://github.com/Cratis/Components/issues/353)). +- Set `focusMode={ToolbarFocusMode.SingleTabStop}` for the WAI-ARIA toolbar pattern: the toolbar's own tools (`ToolbarButton`, folder and fan-out triggers) share one Tab stop, which starts at the first available tool, follows the arrow keys, and returns to the last focused tool. When that tool is disabled, hidden, or removed, the Tab stop moves to the first available tool. Controls you place in the toolbar yourself, widgets such as inputs and sliders, and tools with an explicit `pt.root.tabIndex` keep their own Tab stop, and the arrow keys still reach them. Until the toolbar has chosen its active tool (for example, in server-rendered markup before hydration), every tool keeps its own Tab stop. +- Set `focusMode={ToolbarFocusMode.None}` to keep native Tab behavior without toolbar key handling. Import `ToolbarFocusMode` from `@cratis/components/Toolbar`. `SingleTabStop` becomes the default in the next major release ([#353](https://github.com/Cratis/Components/issues/353)). - Arrows and Home/End inside inputs, sliders, selects, and custom widgets retain their own behavior. Nested toolbars handle their own keys. Key handlers on a tool or `pt.root` can cancel navigation with `preventDefault()` or `stopPropagation()`. - `title` is required on `ToolbarButton` and `ToolbarFolder`, and `tooltip` on `ToolbarFanOutItem`. That text becomes the button's `aria-label` and its tooltip, which appears on hover and on keyboard focus. - Passing `active` sets `aria-pressed` to `true` or `false` as well as the visual state (`data-active`, `data-selected`); omitting `active` leaves it unset. See [Active state](active-state.md). diff --git a/Source/Common/ActionMenubar.stories.tsx b/Source/Common/ActionMenubar.stories.tsx index e7f287cd..9619c34a 100644 --- a/Source/Common/ActionMenubar.stories.tsx +++ b/Source/Common/ActionMenubar.stories.tsx @@ -38,6 +38,30 @@ export const Default: Story = { }, }; +/** One Tab stop for the actions: arrows move it, and Tab leaves the menubar from the last focused action. */ +export const SingleTabStop: Story = { + args: { focusMode: ToolbarFocusMode.SingleTabStop }, + render: (args) => ( +
+ + + +
+ ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + canvas.getByRole('button', { name: 'Before' }).focus(); + await userEvent.tab(); + await expect(canvas.getByRole('button', { name: 'New' })).toHaveFocus(); + await userEvent.keyboard('{ArrowRight}'); + await expect(canvas.getByRole('button', { name: 'Save' })).toHaveFocus(); + await userEvent.tab(); + await expect(canvas.getByRole('button', { name: 'After' })).toHaveFocus(); + await userEvent.tab({ shift: true }); + await expect(canvas.getByRole('button', { name: 'Save' })).toHaveFocus(); + }, +}; + export const ArrowsWithWidgets: Story = { args: { focusMode: ToolbarFocusMode.Arrows, diff --git a/Source/Common/ActionMenubar.tsx b/Source/Common/ActionMenubar.tsx index aa290f16..c42c34ca 100644 --- a/Source/Common/ActionMenubar.tsx +++ b/Source/Common/ActionMenubar.tsx @@ -5,6 +5,7 @@ import { Fragment, type ReactNode } from 'react'; import { Button, type ButtonParts, type ButtonSeverity, type ButtonTone } from './Button'; import { ToolbarFocusMode } from './ToolbarFocusMode'; import { useToolbarKeyboardNavigation } from './useToolbarKeyboardNavigation'; +import { ToolbarRovingContext, useRovingTool } from './ToolbarRovingContext'; /** A single action in an {@link ActionMenubar}. */ export interface ActionMenuItem { @@ -56,6 +57,28 @@ const buttonToneForSeverity: Record = { contrast: 'neutral', }; +/** One Components-owned action button, which takes part in the menubar's single Tab stop. */ +const ActionMenubarButton = ({ item, pt }: { item: ActionMenuItem; pt: ActionMenubarProps['pt'] }) => { + // The consumer's pt.root tab index and focus handler are composed, because props passed + // directly to Button take precedence over pt.root. + const rovingTool = useRovingTool(pt?.root?.tabIndex, pt?.root?.onFocus); + return ( + + + + + + + + + + + ); +}; + +beforeEach(() => { + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); +}); + +describe('when a toolbar uses a single Tab stop', () => { + it('should give only the first of its own tools a Tab stop and keep consumer tab indexes', async () => { + await render(); + expect(tabIndexes()).to.deep.equal(['0', '-1', '-1', '-1']); + }); + + it('should enter at the active tool, skip its other tools, and keep the input a separate Tab stop', async () => { + await render(); + const user = userEvent.setup(); + container.querySelector('button')!.focus(); + await user.tab(); + expect(document.activeElement?.getAttribute('aria-label')).to.equal('Draw'); + await user.tab(); + expect(document.activeElement?.getAttribute('aria-label')).to.equal('Size'); + await user.tab(); + expect(document.activeElement?.textContent).to.equal('After'); + }); + + it('should move the Tab stop with arrow keys and return to the last focused tool', async () => { + await render(); + const [draw] = toolButtons(); + draw.focus(); + await key(draw, 'ArrowRight'); + expect(document.activeElement?.getAttribute('aria-label')).to.equal('Erase'); + expect(tabIndexes()).to.deep.equal(['-1', '0', '-1', '-1']); + const user = userEvent.setup(); + container.querySelector('button')!.focus(); + await user.tab(); + expect(document.activeElement?.getAttribute('aria-label')).to.equal('Erase'); + }); + + it('should move the Tab stop to the next available tool when the active tool becomes unavailable', async () => { + await render(); + await act(async () => disableFirst(true)); + expect(tabIndexes()).to.deep.equal(['-1', '0', '-1', '-1']); + }); + + // The tooltip trigger gives each tool tabindex=0 in the default mode, as before this change. + it('should render the default tab indexes in the default mode', async () => { + await render(); + expect(tabIndexes()).to.deep.equal(['0', '0', '0', '-1']); + }); + + it('should restore the default tab indexes when switching back to the default mode', async () => { + await render(); + await render(); + expect(tabIndexes()).to.deep.equal(['0', '0', '0', '-1']); + }); +}); + +describe('when an action menubar uses a single Tab stop', () => { + it('should give only its first action a Tab stop and move it with arrow keys', async () => { + await render( + , + ); + const actions = () => Array.from(container.querySelectorAll('button')); + expect(actions().map(action => action.getAttribute('tabindex'))).to.deep.equal(['0', '-1', '-1']); + actions()[0].focus(); + await key(actions()[0], 'End'); + expect(document.activeElement?.textContent).to.contain('Delete'); + expect(actions().map(action => action.getAttribute('tabindex'))).to.deep.equal(['-1', '-1', '0']); + }); +}); diff --git a/Storybook/scripts/storybook-inventory.json b/Storybook/scripts/storybook-inventory.json index e3e8a44b..e1ad8e89 100644 --- a/Storybook/scripts/storybook-inventory.json +++ b/Storybook/scripts/storybook-inventory.json @@ -138,6 +138,7 @@ "common-actionmenubar--custom-parts", "common-actionmenubar--default", "common-actionmenubar--disabled-action", + "common-actionmenubar--single-tab-stop", "common-button--ghost", "common-button--icon-only", "common-button--label", @@ -370,6 +371,7 @@ "common-actionmenubar--custom-parts", "common-actionmenubar--default", "common-actionmenubar--disabled-action", + "common-actionmenubar--single-tab-stop", "common-breadcrumbs--custom-separator", "common-breadcrumbs--default", "common-breadcrumbs--marks-the-current-page", From dc46973f5d92c4bb4aeb2f608b825d1e51a6137a Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 07:02:55 +0200 Subject: [PATCH 2/9] Wait for the dialog instead of a fixed delay in the dismissal specs --- .../for_CommandDialog/when_dismissal_is_configured.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts b/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts index 95e2fd7d..fb160dbe 100644 --- a/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts +++ b/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts @@ -7,6 +7,7 @@ import React from 'react'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { vi } from 'vitest'; +import { waitFor } from '@testing-library/dom'; import { CommandDialog } from '../CommandDialog'; import { CratisComponentsProvider } from '../../Common/CratisComponentsProvider'; @@ -81,8 +82,9 @@ describe('when dismissal is configured on a command dialog', () => { ), ); }); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 300)); + // Wait for the dialog itself rather than for a fixed delay. + await waitFor(() => { + if (!document.querySelector('[role="dialog"]')) throw new Error('The dialog has not opened yet.'); }); }; From 92fcde27f5052138a626d6e999918d293c7c8f2e Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 07:05:10 +0200 Subject: [PATCH 3/9] Let hosts add content under messages and in the chat sidebar header ChatConversation.renderMessageExtra renders host content under a message body, for reactions or a failed-reply notice, and ChatSidebar forwards it. The sidebar's renderHeaderActions renders content between the title and the close button and receives the open topic. Neither adds markup when unused. --- Documentation/Chat/index.md | 21 ++++++++ Source/Chat/ChatConversation.tsx | 10 ++++ Source/Chat/ChatSidebar.tsx | 12 ++++- .../when_rendering_message_extras.ts | 42 ++++++++++++++++ .../when_rendering_header_actions.ts | 49 +++++++++++++++++++ 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 Source/Chat/for_ChatConversation/when_rendering_message_extras.ts create mode 100644 Source/Chat/for_ChatSidebar/when_rendering_header_actions.ts diff --git a/Documentation/Chat/index.md b/Documentation/Chat/index.md index a52c3898..0a3c9545 100644 --- a/Documentation/Chat/index.md +++ b/Documentation/Chat/index.md @@ -97,6 +97,8 @@ export const Workspace = () => { | `onRequestTopicName` / `isTopicUnnamed` | callbacks | — | The host-side naming contract. | | `selectedTopicId` / `onTopicSelected` | `ChatIdentifier \| null`, callback | Internal | Owns or observes the open topic. | | `authorOf`, `renderAvatar`, `renderAuthorName`, `buildAvatarUrl` | callbacks | — | Author resolution and rendering. Without `authorOf`, the id is shown as the name. | +| `renderMessageExtra` | `(message) => ReactNode` | — | Content rendered under a message's body, such as reactions or a failed-reply notice. | +| `renderHeaderActions` | `(openTopic) => ReactNode` | — | Content rendered in the header between the title and the close button, such as a rename control. | | `actions`, `quickReply` | `ChatMessageAction[]`, `boolean` | —, `true` | Message actions and quick reply; see [Message actions](./message-actions.md). | | `topicActions` | `ChatTopicAction[]` | — | Actions beside available topics; see [Topic actions](./topic-actions.md). | | `mentionCandidates` / `resolveMentionCandidates` | array or callback | — | See [Mentions and emoji](./mentions-and-emoji.md). Omit both to turn mentions off. | @@ -184,6 +186,25 @@ The built-in avatar shows initials on a color derived from the id. It shows an i /> ``` +## Add your own content to messages and the header + +The chat family stays small, so richer per-message features belong to your application. `renderMessageExtra` renders content under each message's body: reactions, a notice for a reply that failed, or anything else keyed off your own message type. `ChatSidebar` forwards it to the conversation. `renderHeaderActions` renders content in the sidebar header, next to the title; it receives the open topic, or `undefined` while the topic list is shown. Returning `null` renders nothing, and the chat's markup is unchanged when you leave both unset. + +```tsx +type AppMessage = ChatMessage & { failed?: boolean; reactions?: string[] }; + + + renderMessageExtra={(message) => + message.failed ?

The agent could not answer.

: + message.reactions?.length ? : null} + renderHeaderActions={(openTopic) => + openTopic ? : null} + /* ...the rest of your props */ +/> +``` + +`MyReactions` and `MyRenameButton` are your own components. The content you render keeps its own semantics and styling; give interactive controls accessible names. + ## Rendering a message body directly `ChatConversation` uses `ChatMessageBody` internally. Import it directly when an application-owned message list, notification, or transcript needs the same mention rendering without the rest of the conversation UI. diff --git a/Source/Chat/ChatConversation.tsx b/Source/Chat/ChatConversation.tsx index 1bef7a29..97f911bf 100644 --- a/Source/Chat/ChatConversation.tsx +++ b/Source/Chat/ChatConversation.tsx @@ -118,6 +118,14 @@ export interface ChatConversationProps ReactNode; + /** + * Renders host content under a message's body, for example reactions or a failed-reply notice. + * Nothing extra is rendered when it returns null or undefined. + * @param message The message being rendered. + * @returns What to render under the message body. + */ + renderMessageExtra?: (message: TMessage) => ReactNode; + /** The host's own actions, offered as buttons on every message each is available for. */ actions?: ChatMessageAction[]; @@ -228,6 +236,7 @@ export const ChatConversation = ({ authorOf, renderAvatar, renderAuthorName, + renderMessageExtra, actions, mentionCandidates, resolveMentionCandidates, @@ -377,6 +386,7 @@ export const ChatConversation = ({ )} + {renderMessageExtra?.(message)} {showTimestamp && ( {relativeTimestamp( diff --git a/Source/Chat/ChatSidebar.tsx b/Source/Chat/ChatSidebar.tsx index 3f3ae177..c7205208 100644 --- a/Source/Chat/ChatSidebar.tsx +++ b/Source/Chat/ChatSidebar.tsx @@ -2,7 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. import { useEffect, useRef, useState } from 'react'; -import type { ButtonHTMLAttributes, HTMLAttributes } from 'react'; +import type { ButtonHTMLAttributes, HTMLAttributes, ReactNode } from 'react'; import { createPortal } from 'react-dom'; import { Modal, ModalOverlay } from 'react-aria-components'; import type { ChatConversationLabels, ChatConversationProps } from './ChatConversation'; @@ -89,6 +89,14 @@ export interface ChatSidebarProps< ChatConversationProps, 'messages' | 'onSendMessage' | 'labels' | 'className' | 'status' > { + /** + * Renders host content in the header, between the title and the close button, for example a + * rename control for the open topic. Nothing is rendered when it returns null or undefined. + * @param openTopic The open topic, or undefined while the topic list is shown. + * @returns What to render in the header. + */ + renderHeaderActions?: (openTopic: TTopic | undefined) => ReactNode; + /** Whether the sidebar is open. */ open: boolean; @@ -230,6 +238,7 @@ export const ChatSidebar = < topics, messages, topicActions, + renderHeaderActions, topicsStatus, messagesStatus, selectedTopicId, @@ -379,6 +388,7 @@ export const ChatSidebar = < > {title} + {renderHeaderActions?.(openTopic)} + + } title='Draw' /> + } title='Erase' /> + + + + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + canvas.getByRole('button', { name: 'Before' }).focus(); + await userEvent.tab(); + await expect(canvas.getByRole('button', { name: 'Draw' })).toHaveFocus(); + await userEvent.keyboard('{ArrowRight}'); + await expect(canvas.getByRole('button', { name: 'Erase' })).toHaveFocus(); + await userEvent.tab(); + await expect(canvas.getByRole('textbox', { name: 'Brush size' })).toHaveFocus(); + await userEvent.tab(); + await expect(canvas.getByRole('button', { name: 'After' })).toHaveFocus(); + await userEvent.tab({ shift: true }); + await userEvent.tab({ shift: true }); + await expect(canvas.getByRole('button', { name: 'Erase' })).toHaveFocus(); + }, +}; + export const WithActiveButton: Story = { render: () => { const ActiveDemo = () => { diff --git a/Source/Toolbar/ToolbarFanOutItem.tsx b/Source/Toolbar/ToolbarFanOutItem.tsx index 337e8fcd..dff78d57 100644 --- a/Source/Toolbar/ToolbarFanOutItem.tsx +++ b/Source/Toolbar/ToolbarFanOutItem.tsx @@ -80,6 +80,11 @@ export const ToolbarFanOutItem = ({ const containerRef = useRef(null); const triggerRef = useRef(null); const rovingTrigger = useRovingTool(pt?.trigger?.tabIndex, pt?.trigger?.onFocus); + const rovingTriggerRef = rovingTrigger.ref; + const setTriggerRef = useCallback((element: HTMLButtonElement | null) => { + triggerRef.current = element; + if (typeof rovingTriggerRef === 'function') rovingTriggerRef(element); + }, [rovingTriggerRef]); const panelRef = useRef(null); const generatedPanelId = useId(); const panelId = pt?.panel?.id ?? generatedPanelId; @@ -172,10 +177,7 @@ export const ToolbarFanOutItem = ({ {...pt?.trigger} tabIndex={rovingTrigger.tabIndex} onFocus={rovingTrigger.onFocus} - ref={(element: HTMLButtonElement | null) => { - triggerRef.current = element; - if (typeof rovingTrigger.ref === 'function') rovingTrigger.ref(element); - }} + ref={setTriggerRef} type='button' aria-label={tooltip} aria-expanded={isExpanded} diff --git a/Source/Toolbar/ToolbarFolder.tsx b/Source/Toolbar/ToolbarFolder.tsx index b79e9b30..d03eb581 100644 --- a/Source/Toolbar/ToolbarFolder.tsx +++ b/Source/Toolbar/ToolbarFolder.tsx @@ -10,6 +10,7 @@ import { useId, useMemo, useRef, + useCallback, useState, } from 'react'; import { useRovingTool } from '../Common/ToolbarRovingContext'; @@ -94,6 +95,11 @@ export const ToolbarFolder = ({ const containerRef = useRef(null); const triggerRef = useRef(null); const rovingTrigger = useRovingTool(pt?.trigger?.tabIndex, pt?.trigger?.onFocus); + const rovingTriggerRef = rovingTrigger.ref; + const setTriggerRef = useCallback((element: HTMLButtonElement | null) => { + triggerRef.current = element; + if (typeof rovingTriggerRef === 'function') rovingTriggerRef(element); + }, [rovingTriggerRef]); const generatedPanelId = useId(); const panelId = pt?.panel?.id ?? generatedPanelId; @@ -169,10 +175,7 @@ export const ToolbarFolder = ({ {...pt?.trigger} tabIndex={rovingTrigger.tabIndex} onFocus={rovingTrigger.onFocus} - ref={(element: HTMLButtonElement | null) => { - triggerRef.current = element; - if (typeof rovingTrigger.ref === 'function') rovingTrigger.ref(element); - }} + ref={setTriggerRef} type='button' aria-label={title} aria-expanded={isExpanded} diff --git a/Source/Toolbar/for_Toolbar/when_using_a_single_tab_stop.tsx b/Source/Toolbar/for_Toolbar/when_using_a_single_tab_stop.tsx index b9153043..01781cba 100644 --- a/Source/Toolbar/for_Toolbar/when_using_a_single_tab_stop.tsx +++ b/Source/Toolbar/for_Toolbar/when_using_a_single_tab_stop.tsx @@ -3,13 +3,14 @@ // @vitest-environment jsdom -import { act, useState } from 'react'; +import { act, StrictMode, useState } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import userEvent from '@testing-library/user-event'; import { afterEach, beforeEach, describe, it } from 'vitest'; import { expect } from 'chai'; import { Toolbar } from '../Toolbar'; import { ToolbarButton } from '../ToolbarButton'; +import { ToolbarFolder } from '../ToolbarFolder'; import { ActionMenubar } from '../../Common/ActionMenubar'; import { ToolbarFocusMode } from '../../Common/ToolbarFocusMode'; @@ -107,6 +108,80 @@ describe('when a toolbar uses a single Tab stop', () => { }); }); +// State that changes inside a tool's own wrapper does not re-render the toolbar; the toolbar must +// still notice when its active tool is no longer available. +let changeFirst: (change: 'optOut' | 'disable' | 'remove' | undefined) => void = () => undefined; +const WrappedFirstTool = () => { + const [change, setChange] = useState<'optOut' | 'disable' | 'remove' | undefined>(undefined); + changeFirst = setChange; + if (change === 'remove') return null; + return ( + + ); +}; + +describe('when the active tool changes inside its own wrapper', () => { + for (const change of ['optOut', 'disable', 'remove'] as const) { + it(`should move the Tab stop to the next tool after ${change}`, async () => { + await render( + + + + , + ); + expect(container.querySelector('[aria-label="Draw"]')!.getAttribute('tabindex')).to.equal('0'); + await act(async () => changeFirst(change)); + await act(async () => { await Promise.resolve(); }); + expect(container.querySelector('[aria-label="Erase"]')!.getAttribute('tabindex')).to.equal('0'); + }); + } +}); + +describe('when a toolbar with a folder uses a single Tab stop', () => { + it('should make the folder trigger part of the single Tab stop', async () => { + await render( + + + + + + , + ); + expect(container.querySelector('[aria-label="Shapes"]')!.getAttribute('tabindex')).to.equal('-1'); + const draw = container.querySelector('[aria-label="Draw"]')!; + draw.focus(); + await key(draw, 'ArrowDown'); + expect(document.activeElement?.getAttribute('aria-label')).to.equal('Shapes'); + expect(container.querySelector('[aria-label="Shapes"]')!.getAttribute('tabindex')).to.equal('0'); + }); +}); + +describe('when a consumer handles focus on a tool', () => { + it('should call the consumer handler and still move the Tab stop', async () => { + let focused = 0; + await render( + + + { focused++; } } }} /> + , + ); + await act(async () => container.querySelector('[aria-label="Erase"]')!.focus()); + expect(focused).to.equal(1); + expect(container.querySelector('[aria-label="Erase"]')!.getAttribute('tabindex')).to.equal('0'); + }); +}); + +describe('when a single Tab stop toolbar renders in StrictMode', () => { + it('should still give exactly one tool the Tab stop', async () => { + await render(); + expect(tabIndexes()).to.deep.equal(['0', '-1', '-1', '-1']); + }); +}); + describe('when an action menubar uses a single Tab stop', () => { it('should give only its first action a Tab stop and move it with arrow keys', async () => { await render( diff --git a/Storybook/scripts/storybook-inventory.json b/Storybook/scripts/storybook-inventory.json index e1ad8e89..41988fe3 100644 --- a/Storybook/scripts/storybook-inventory.json +++ b/Storybook/scripts/storybook-inventory.json @@ -619,6 +619,7 @@ "toolbar-toolbar--layout-for-editor-modules", "toolbar-toolbar--layout-with-global-and-editor-regions", "toolbar-toolbar--layout-with-smooth-editor-transitions", + "toolbar-toolbar--single-tab-stop", "toolbar-toolbar--with-active-button", "toolbar-toolbar--with-contexts", "toolbar-toolbar--with-editable-tool", From aa71f091f4904bcbd5b798cb28ec1b6a5316e40b Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 07:31:17 +0200 Subject: [PATCH 5/9] Let hosts control DataTableCore sort, filter, and search state sort with onSortChange, filters with onFilter, and globalFilter with onGlobalFilterChange are controllable; undefined keeps today's internal state. rowProcessing=None renders rows as given when the source already filtered and sorted them. First step of #178; server mode for the bound tables follows the Arc fixes. --- Documentation/DataTables/index.md | 4 + Source/DataTables/DataTableCore.tsx | 56 ++++++++--- Source/DataTables/DataTableRowProcessing.ts | 13 +++ Source/DataTables/DataTableSort.ts | 12 +++ Source/DataTables/DataTableSortDirection.ts | 10 ++ ...when_controlling_sort_and_filter_state.tsx | 97 +++++++++++++++++++ Source/DataTables/index.ts | 3 + Source/DataTables/useControllableState.ts | 26 +++++ 8 files changed, 205 insertions(+), 16 deletions(-) create mode 100644 Source/DataTables/DataTableRowProcessing.ts create mode 100644 Source/DataTables/DataTableSort.ts create mode 100644 Source/DataTables/DataTableSortDirection.ts create mode 100644 Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx create mode 100644 Source/DataTables/useControllableState.ts diff --git a/Documentation/DataTables/index.md b/Documentation/DataTables/index.md index 0520bd22..74c1993a 100644 --- a/Documentation/DataTables/index.md +++ b/Documentation/DataTables/index.md @@ -15,6 +15,10 @@ The DataTables module provides a semantic local-array table plus specialized Arc `DataTableCore` accepts a `status` from `DataTableStatus` (`Ready`, `Loading`, `Failed`, or `Unauthorized`) (import `DataTableStatus` from `@cratis/components/DataTables`); it defaults to `Ready`. For a loading table with no rows, it renders a status row; with rows, it retains them and marks the table busy. Failed or unauthorized states render an alert row. Override the text with `loadingMessage`, `failureMessage`, and `unauthorizedMessage` or configure `CratisComponentsProvider`'s `messages.dataTable`. +`DataTableCore` keeps its own sort, column filters, and search text unless you control them. Pass `sort` with `onSortChange`, `filters` with `onFilter`, and `globalFilter` with `onGlobalFilterChange` to own that state, for example to persist it or to send it to the server. `sort={null}` means not sorted; leaving a prop undefined lets the table keep that piece of state itself, and the change callbacks report every change either way. Import `DataTableSort`, `DataTableSortDirection`, and `DataTableRowProcessing` from `@cratis/components/DataTables`. + +When the rows already reflect that state, because your query filtered and sorted them on the server, set `rowProcessing={DataTableRowProcessing.None}` so the table renders the rows as given instead of filtering and sorting them a second time. The default, `DataTableRowProcessing.Loaded`, filters and sorts the rows the table was given. The bound query tables do not take these props yet; server-side sorting and filtering for them is tracked in [#178](https://github.com/Cratis/Components/issues/178). + ## When to Use Use DataTableCore when: diff --git a/Source/DataTables/DataTableCore.tsx b/Source/DataTables/DataTableCore.tsx index 35277c7a..ba5bac61 100644 --- a/Source/DataTables/DataTableCore.tsx +++ b/Source/DataTables/DataTableCore.tsx @@ -6,7 +6,6 @@ import React, { useId, useMemo, useRef, - useState, type CSSProperties, type HTMLAttributes, type ReactNode, @@ -27,6 +26,10 @@ import { } from './DataTableFilterMeta'; import { resolveDataTableFilterMatcher } from './DataTableFilterMatcherRegistry'; import { DataTableStatus } from './DataTableStatus'; +import type { DataTableSort } from './DataTableSort'; +import { DataTableSortDirection } from './DataTableSortDirection'; +import { DataTableRowProcessing } from './DataTableRowProcessing'; +import { useControllableState } from './useControllableState'; /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -124,8 +127,24 @@ export interface DataTableCoreProps { globalSearchAriaLabel?: string; /** Initial per-field filter constraints. */ defaultFilters?: DataTableFilterMeta; + /** Controlled per-field filter constraints. Leave undefined to let the table keep its own. */ + filters?: DataTableFilterMeta; /** Invoked when applied field filters change. */ onFilter?: (filters: DataTableFilterMeta) => void; + /** Controlled search text. Leave undefined to let the table keep its own. */ + globalFilter?: string; + /** Invoked when the search text changes. */ + onGlobalFilterChange?: (globalFilter: string) => void; + /** Controlled sort; null means not sorted. Leave undefined to let the table keep its own. */ + sort?: DataTableSort | null; + /** Invoked when a sortable column header changes the sort. */ + onSortChange?: (sort: DataTableSort | null) => void; + /** + * Whether the table filters and sorts the rows it is given (default: + * {@link DataTableRowProcessing.Loaded}). Use {@link DataTableRowProcessing.None} when the + * source already applied the controlled filter and sort state. + */ + rowProcessing?: DataTableRowProcessing; /** Enables the bounded scroll container. */ scrollable?: boolean; /** Scroll-container maximum height. */ @@ -302,7 +321,13 @@ export const DataTableCore = ({ globalSearchPlaceholder, globalSearchAriaLabel, defaultFilters, + filters: filtersProp, onFilter, + globalFilter: globalFilterProp, + onGlobalFilterChange, + sort: sortProp, + onSortChange, + rowProcessing = DataTableRowProcessing.Loaded, scrollable, scrollHeight, className, @@ -331,14 +356,14 @@ export const DataTableCore = ({ const sortDescendingIcon = icon('sortDescending', '▼'); const columns = useColumns(children); const selectionGroupName = useId(); - const [filters, setFilters] = useState(defaultFilters ?? {}); - const [globalFilter, setGlobalFilter] = useState(''); - const [sort, setSort] = useState<{ - field: string; - direction: 'ascending' | 'descending'; - }>(); + const [filters, setFilters] = useControllableState(filtersProp, defaultFilters ?? {}, onFilter); + const [globalFilter, setGlobalFilter] = useControllableState(globalFilterProp, '', onGlobalFilterChange); + const [sort, setSort] = useControllableState(sortProp, null, onSortChange); const filteredRows = useMemo(() => { + if (rowProcessing === DataTableRowProcessing.None) { + return data.map((row, loadedIndex) => ({ row, loadedIndex })); + } const term = globalFilter.trim().toLocaleLowerCase(); const rows = data .map((row, loadedIndex) => ({ row, loadedIndex })) @@ -364,7 +389,7 @@ export const DataTableCore = ({ ); return sort.direction === 'ascending' ? comparison : -comparison; }); - }, [data, filters, globalFilter, globalFilterFields, sort]); + }, [data, filters, globalFilter, globalFilterFields, sort, rowProcessing]); const updateFilter = ( field: string, @@ -374,7 +399,6 @@ export const DataTableCore = ({ if (constraint) next[field] = constraint; else delete next[field]; setFilters(next); - onFilter?.(next); }; const activateRow = ( @@ -619,17 +643,17 @@ export const DataTableCore = ({ Boolean(ariaSort) || undefined } onClick={() => - setSort((current) => ({ + setSort({ field: column.props .field as string, direction: - current?.field === + sort?.field === column.props.field && - current?.direction === - 'ascending' - ? 'descending' - : 'ascending', - })) + sort?.direction === + DataTableSortDirection.Ascending + ? DataTableSortDirection.Descending + : DataTableSortDirection.Ascending, + }) } > {column.props.header} diff --git a/Source/DataTables/DataTableRowProcessing.ts b/Source/DataTables/DataTableRowProcessing.ts new file mode 100644 index 00000000..925ba028 --- /dev/null +++ b/Source/DataTables/DataTableRowProcessing.ts @@ -0,0 +1,13 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +/** Whether a data table filters and sorts the rows it is given. */ +export enum DataTableRowProcessing { + /** Filter and sort the rows the table was given, such as the loaded page (default). */ + Loaded = 'loaded', + /** + * Render the rows exactly as given. Use when the source already applied the table's filter and + * sort state, for example a server query driven by the controlled `filters` and `sort` props. + */ + None = 'none', +} diff --git a/Source/DataTables/DataTableSort.ts b/Source/DataTables/DataTableSort.ts new file mode 100644 index 00000000..232219c0 --- /dev/null +++ b/Source/DataTables/DataTableSort.ts @@ -0,0 +1,12 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import type { DataTableSortDirection } from './DataTableSortDirection'; + +/** The column a data table is sorted by, and in which direction. */ +export interface DataTableSort { + /** The sorted column's field. */ + field: string; + /** The sort direction. */ + direction: DataTableSortDirection; +} diff --git a/Source/DataTables/DataTableSortDirection.ts b/Source/DataTables/DataTableSortDirection.ts new file mode 100644 index 00000000..164e0be6 --- /dev/null +++ b/Source/DataTables/DataTableSortDirection.ts @@ -0,0 +1,10 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +/** The direction a data table column is sorted in. */ +export enum DataTableSortDirection { + /** Smallest value first. */ + Ascending = 'ascending', + /** Largest value first. */ + Descending = 'descending', +} diff --git a/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx b/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx new file mode 100644 index 00000000..3392baf4 --- /dev/null +++ b/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx @@ -0,0 +1,97 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// @vitest-environment jsdom + +import { expect } from 'chai'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, it } from 'vitest'; +import { Column } from '../Column'; +import { DataTableCore, type DataTableCoreProps } from '../DataTableCore'; +import type { DataTableSort } from '../DataTableSort'; +import { DataTableSortDirection } from '../DataTableSortDirection'; +import { DataTableRowProcessing } from '../DataTableRowProcessing'; +import { DataTableFilterMatchMode } from '../DataTableFilterMeta'; + +interface Product { + id: number; + name: string; +} + +const data: Product[] = [ + { id: 1, name: 'Charlie' }, + { id: 2, name: 'Alpha' }, + { id: 3, name: 'Bravo' }, +]; + +let container: HTMLDivElement; +let root: Root; +const names = () => Array.from(container.querySelectorAll('tbody tr')).map(row => row.textContent?.trim()); +const render = async (props: Partial>) => { + await act(async () => { + root.render( + data={data} dataKey='id' emptyMessage='None' globalFilterFields={['name']} {...props}> + field='name' header='Name' sortable /> + , + ); + }); +}; +const clickSort = async () => { + await act(async () => container.querySelector('[data-cratis-part="sort"]')!.click()); +}; + +beforeEach(() => { + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); +}); + +describe('when controlling sort and filter state', () => { + it('should sort by the controlled sort and report header clicks without changing it', async () => { + const reported: (DataTableSort | null)[] = []; + await render({ sort: { field: 'name', direction: DataTableSortDirection.Descending }, onSortChange: sort => reported.push(sort) }); + expect(names()).to.deep.equal(['Charlie', 'Bravo', 'Alpha']); + await clickSort(); + expect(reported).to.deep.equal([{ field: 'name', direction: DataTableSortDirection.Ascending }]); + expect(names()).to.deep.equal(['Charlie', 'Bravo', 'Alpha']); + }); + + it('should show rows unsorted when the controlled sort is null', async () => { + await render({ sort: null }); + expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); + }); + + it('should keep its own sort and still report it when uncontrolled', async () => { + const reported: (DataTableSort | null)[] = []; + await render({ onSortChange: sort => reported.push(sort) }); + await clickSort(); + expect(names()).to.deep.equal(['Alpha', 'Bravo', 'Charlie']); + expect(reported).to.deep.equal([{ field: 'name', direction: DataTableSortDirection.Ascending }]); + }); + + it('should filter by the controlled search text', async () => { + await render({ globalFilter: 'bra' }); + expect(names()).to.deep.equal(['Bravo']); + expect(container.querySelector('input[type="search"], input')!.value).to.equal('bra'); + }); + + it('should filter by the controlled column filters', async () => { + await render({ filters: { name: { value: 'Alpha', matchMode: DataTableFilterMatchMode.Equals } } }); + expect(names()).to.deep.equal(['Alpha']); + }); + + it('should render rows as given when row processing is off', async () => { + await render({ + rowProcessing: DataTableRowProcessing.None, + sort: { field: 'name', direction: DataTableSortDirection.Ascending }, + globalFilter: 'bra', + }); + expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); + }); +}); diff --git a/Source/DataTables/index.ts b/Source/DataTables/index.ts index 6ade62a2..14f199e1 100644 --- a/Source/DataTables/index.ts +++ b/Source/DataTables/index.ts @@ -10,6 +10,9 @@ export * from './ColumnFilterMenu'; export * from './TablePaginator'; export * from './DataTableSelectionChangeEvent'; export * from './DataTableFilterMeta'; +export type { DataTableSort } from './DataTableSort'; +export { DataTableSortDirection } from './DataTableSortDirection'; +export { DataTableRowProcessing } from './DataTableRowProcessing'; export { registerDataTableFilterMatcher, resolveDataTableFilterMatcher, diff --git a/Source/DataTables/useControllableState.ts b/Source/DataTables/useControllableState.ts new file mode 100644 index 00000000..13d44b85 --- /dev/null +++ b/Source/DataTables/useControllableState.ts @@ -0,0 +1,26 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +import { useCallback, useState } from 'react'; + +/** + * State that the owner controls when it passes a value, and that the component keeps itself when + * the value is undefined. Changes are always reported through the change handler. + * @param value The controlled value, or undefined to let the component keep its own state. + * @param defaultValue The initial value while uncontrolled. + * @param onChange Called with every new value. + * @returns The current value and a setter. + */ +export const useControllableState = ( + value: TValue | undefined, + defaultValue: TValue, + onChange: ((next: TValue) => void) | undefined, +): [TValue, (next: TValue) => void] => { + const [internal, setInternal] = useState(defaultValue); + const controlled = value !== undefined; + const set = useCallback((next: TValue) => { + if (!controlled) setInternal(next); + onChange?.(next); + }, [controlled, onChange]); + return [controlled ? value : internal, set]; +}; From 88ea52ef0ae1a3b3f26a8c588767632d8b10d27d Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 08:10:55 +0200 Subject: [PATCH 6/9] Keep tools outside the toolbar's scope reachable in SingleTabStop mode A tool rendered through a portal or under a nested role=toolbar registered with the toolbar but could never become active, so it stayed at -1. Such tools now keep their native Tab stop. A ResizeObserver catches hiding by stylesheet alone, reconcile checks only the active tool on most renders, and the docs state that an explicit tabIndex of -1 leaves arrow navigation. --- Documentation/Toolbar/index.md | 2 +- Source/Common/ToolbarFocusMode.ts | 4 +- Source/Common/ToolbarRovingContext.ts | 10 +- Source/Common/useToolbarKeyboardNavigation.ts | 111 +++++++++--- Source/Toolbar/Toolbar.stories.tsx | 2 +- ..._sit_at_the_edges_of_a_single_tab_stop.tsx | 165 ++++++++++++++++++ 6 files changed, 262 insertions(+), 32 deletions(-) create mode 100644 Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx diff --git a/Documentation/Toolbar/index.md b/Documentation/Toolbar/index.md index 1e2d6901..14267dd5 100644 --- a/Documentation/Toolbar/index.md +++ b/Documentation/Toolbar/index.md @@ -21,7 +21,7 @@ import { Toolbar, ToolbarButton } from '@cratis/components/Toolbar'; - The root renders `role='toolbar'`, `aria-orientation`, and an accessible name. The name falls back to the provider's `messages.toolbar.label`, then `Tools`; pass `aria-label` or `aria-labelledby` to name each toolbar. - `focusMode` defaults to `ToolbarFocusMode.Arrows`: Up/Down move between tools in the default vertical orientation; a horizontal toolbar uses Left/Right (reversed in RTL). Home/End move to the first/last available tool in DOM order. Navigation skips disabled, hidden, and inert tools and stops at either end; it does not wrap. Each tool remains its own Tab stop, so Tab and Shift+Tab work as before. -- Set `focusMode={ToolbarFocusMode.SingleTabStop}` for the WAI-ARIA toolbar pattern: the toolbar's own tools (`ToolbarButton`, folder and fan-out triggers) share one Tab stop, which starts at the first available tool, follows the arrow keys, and returns to the last focused tool. When that tool is disabled, hidden, or removed, the Tab stop moves to the first available tool. Tools in an open folder or fan-out panel belong to the same Tab stop, so you reach them with the arrow keys rather than Tab. Controls you place in the toolbar yourself, widgets such as inputs and sliders, a `ToolbarButton` with an explicit `pt.root.tabIndex`, and a folder or fan-out trigger with an explicit `pt.trigger.tabIndex` keep their own Tab stop, and the arrow keys still reach them. Until the toolbar has chosen its active tool (for example, in server-rendered markup before hydration), every tool keeps its own Tab stop. +- Set `focusMode={ToolbarFocusMode.SingleTabStop}` for the WAI-ARIA toolbar pattern: the toolbar's own tools (`ToolbarButton`, folder and fan-out triggers) share one Tab stop, which starts at the first available tool, follows the arrow keys, and returns to the last focused tool. When that tool is disabled, hidden, or removed, the Tab stop moves to the first available tool. A tool rendered outside the toolbar's own markup, such as through a portal or inside a nested element with `role="toolbar"`, keeps its own Tab stop. Tools in an open folder or fan-out panel belong to the same Tab stop, so you reach them with the arrow keys rather than Tab. Controls you place in the toolbar yourself, widgets such as inputs and sliders, a `ToolbarButton` with an explicit `pt.root.tabIndex`, and a folder or fan-out trigger with an explicit `pt.trigger.tabIndex` keep their own Tab stop; with a tab index of 0 or higher the arrow keys still reach them, while `-1` removes the tool from both Tab and arrow-key navigation. Until the toolbar has chosen its active tool (for example, in server-rendered markup before hydration), every tool keeps its own Tab stop. - Set `focusMode={ToolbarFocusMode.None}` to keep native Tab behavior without toolbar key handling. Import `ToolbarFocusMode` from `@cratis/components/Toolbar`. `SingleTabStop` becomes the default in the next major release ([#353](https://github.com/Cratis/Components/issues/353)). - Arrows and Home/End inside inputs, sliders, selects, and custom widgets retain their own behavior. Nested toolbars handle their own keys. Key handlers on a tool or `pt.root` can cancel navigation with `preventDefault()` or `stopPropagation()`. - `title` is required on `ToolbarButton` and `ToolbarFolder`, and `tooltip` on `ToolbarFanOutItem`. That text becomes the button's `aria-label` and its tooltip, which appears on hover and on keyboard focus. diff --git a/Source/Common/ToolbarFocusMode.ts b/Source/Common/ToolbarFocusMode.ts index ac546964..64c02dff 100644 --- a/Source/Common/ToolbarFocusMode.ts +++ b/Source/Common/ToolbarFocusMode.ts @@ -8,7 +8,9 @@ export enum ToolbarFocusMode { /** * Navigate with arrows and Home/End, with one Tab stop for the toolbar's own tools that * returns to the last focused tool (the WAI-ARIA toolbar pattern). Controls you add yourself, - * widgets such as inputs and sliders, and tools with an explicit tab index keep their own Tab stop. + * widgets such as inputs and sliders, tools rendered outside the toolbar's own markup (for example + * through a portal), and tools with an explicit tab index keep their own Tab stop. An explicit + * tab index of -1 removes the tool from both Tab and arrow-key navigation. */ SingleTabStop = 'singleTabStop', /** Keep native focus behavior with no toolbar keyboard handling. */ diff --git a/Source/Common/ToolbarRovingContext.ts b/Source/Common/ToolbarRovingContext.ts index 320d4bad..7813c2c4 100644 --- a/Source/Common/ToolbarRovingContext.ts +++ b/Source/Common/ToolbarRovingContext.ts @@ -11,6 +11,11 @@ import { createContext, useCallback, useContext, useId, useSyncExternalStore, ty export interface ToolbarRoving { /** The active tool's key, or null until the toolbar has chosen one. */ getActiveKey(): string | null; + /** + * Whether a tool lies in the toolbar's own DOM scope. A tool rendered elsewhere, for example + * through a portal or under a nested consumer toolbar, keeps its native Tab stop. + */ + isInScope(key: string): boolean; /** Subscribes to changes of the active tool. */ subscribe(listener: () => void): () => void; /** Records the element for a tool, or removes it when the element is null. */ @@ -56,7 +61,9 @@ export const useRovingTool = ( () => { if (!store) return unmanaged; const activeKey = store.getActiveKey(); - return activeKey === null ? undecided : activeKey === key ? active : inactive; + if (activeKey === null) return undecided; + if (!store.isInScope(key)) return unmanaged; + return activeKey === key ? active : inactive; }, // Server-rendered markup keeps native Tab stops until the toolbar chooses its active tool. () => (store ? undecided : unmanaged), @@ -69,6 +76,7 @@ export const useRovingTool = ( store?.activate(key); }, [onFocus, store, key]); if (!store) return { tabIndex: explicitTabIndex, onFocus }; + // A tool outside the toolbar's scope still registers, so the toolbar can notice when it enters. return { ref, tabIndex: state === active ? 0 : state === inactive ? -1 : undefined, diff --git a/Source/Common/useToolbarKeyboardNavigation.ts b/Source/Common/useToolbarKeyboardNavigation.ts index 698c8c1f..d37820a3 100644 --- a/Source/Common/useToolbarKeyboardNavigation.ts +++ b/Source/Common/useToolbarKeyboardNavigation.ts @@ -21,39 +21,69 @@ export const useToolbarKeyboardNavigation = (orientation: Orientation, focusMode const activeKeyRef = useRef(null); const listeners = useRef(new Set<() => void>()); + // Keys of managed tools that lie in this toolbar's own DOM scope. A tool rendered through a + // portal or under a nested consumer toolbar can never be reached from here, so it keeps its + // native Tab stop instead of a -1 the toolbar could never lift. + const inScopeRef = useRef(new Set()); + const resizeObserverRef = useRef(null); + + const isInToolbarScope = useCallback((tool: Element) => { + const root = rootRef.current; + return !!root && tool !== root && root.contains(tool) && tool.closest('[role="toolbar"]') === root; + }, []); + + const isAvailable = useCallback((tool: HTMLElement) => { + const root = rootRef.current; + if (!root || !isInToolbarScope(tool) || tool.matches(':disabled, [disabled], [aria-disabled="true"]')) return false; + if (tool.getAttribute('tabindex') === '-1' && !keyByManaged.current.has(tool)) return false; + for (let element: HTMLElement | null = tool; element && element !== root; element = element.parentElement) { + if (element.hidden || element.hasAttribute('inert') || element.getAttribute('aria-hidden') === 'true') return false; + const style = getComputedStyle(element); + if (style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse') return false; + } + return true; + }, [isInToolbarScope]); + const tools = useCallback(() => { const root = rootRef.current; if (!root) return []; - return Array.from(root.querySelectorAll(toolSelector)).filter(tool => { - if (tool.closest('[role="toolbar"]') !== root || tool.matches(':disabled, [disabled], [aria-disabled="true"]')) return false; - if (tool.getAttribute('tabindex') === '-1' && !keyByManaged.current.has(tool)) return false; - for (let element: HTMLElement | null = tool; element && element !== root; element = element.parentElement) { - if (element.hidden || element.hasAttribute('inert') || element.getAttribute('aria-hidden') === 'true') return false; - const style = getComputedStyle(element); - if (style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse') return false; - } - return true; - }); - }, []); + return Array.from(root.querySelectorAll(toolSelector)).filter(isAvailable); + }, [isAvailable]); + + const notify = useCallback(() => listeners.current.forEach(listener => listener()), []); const setActiveKey = useCallback((key: string | null) => { - if (activeKeyRef.current === key) return; + if (activeKeyRef.current === key) return false; activeKeyRef.current = key; - listeners.current.forEach(listener => listener()); - }, []); + notify(); + return true; + }, [notify]); - // Keep the active tool available: when it is removed, disabled, hidden, or opts out with its - // own tab index, the first available tool the toolbar owns becomes the single Tab stop. This - // only changes the toolbar's own state; tools render their tab index from it. + // Keep the active tool available: when it is removed, disabled, hidden, moved out of scope, or + // opts out with its own tab index, the first available tool the toolbar owns becomes the single + // Tab stop. This only changes the toolbar's own state; tools render their tab index from it. const reconcile = useCallback(() => { if (!singleTabStop) return; - const available = tools(); + let scopeChanged = false; + for (const key of inScopeRef.current) { + if (!managedByKey.current.has(key)) inScopeRef.current.delete(key); + } + for (const [key, element] of managedByKey.current) { + const inside = isInToolbarScope(element); + if (inside === inScopeRef.current.has(key)) continue; + if (inside) inScopeRef.current.add(key); + else inScopeRef.current.delete(key); + scopeChanged = true; + } const current = activeKeyRef.current; const element = current === null ? undefined : managedByKey.current.get(current); - if (element && available.includes(element)) return; - const first = available.find(tool => keyByManaged.current.has(tool)); - setActiveKey(first ? keyByManaged.current.get(first)! : null); - }, [singleTabStop, tools, setActiveKey]); + // Checking the active tool alone is enough on most renders; the full scan runs only when it is gone. + if (!(element && inScopeRef.current.has(current!) && isAvailable(element))) { + const first = tools().find(tool => inScopeRef.current.has(keyByManaged.current.get(tool) ?? '')); + if (setActiveKey(first ? keyByManaged.current.get(first)! : null)) return; + } + if (scopeChanged) notify(); + }, [singleTabStop, isInToolbarScope, isAvailable, tools, setActiveKey, notify]); // Several registrations and mutations in one commit or frame need only one check. const reconcileScheduled = useRef(false); @@ -71,8 +101,13 @@ export const useToolbarKeyboardNavigation = (orientation: Orientation, focusMode const root = rootRef.current; if (!root || !singleTabStop) return; // Only changes on a managed tool or one of its ancestors can make it unavailable. - const affectsManagedTool = (record: MutationRecord) => record.type === 'childList' || - [...keyByManaged.current.keys()].some(tool => record.target === tool || record.target.contains(tool)); + const affectsManagedTool = (record: MutationRecord) => { + if (record.type === 'childList') return true; + for (const tool of keyByManaged.current.keys()) { + if (record.target === tool || record.target.contains(tool)) return true; + } + return false; + }; const observer = new MutationObserver(records => { if (records.some(affectsManagedTool)) scheduleReconcile(); }); @@ -82,29 +117,49 @@ export const useToolbarKeyboardNavigation = (orientation: Orientation, focusMode attributes: true, attributeFilter: ['hidden', 'inert', 'aria-hidden', 'aria-disabled', 'disabled', 'style', 'class'], }); - return () => observer.disconnect(); + // Hiding through a stylesheet alone, such as a media query, changes no attribute; the + // hidden tool's box collapses instead. + if (typeof ResizeObserver !== 'undefined') { + resizeObserverRef.current = new ResizeObserver(scheduleReconcile); + managedByKey.current.forEach(tool => resizeObserverRef.current!.observe(tool)); + } + return () => { + observer.disconnect(); + resizeObserverRef.current?.disconnect(); + resizeObserverRef.current = null; + }; }, [singleTabStop, scheduleReconcile]); const roving = useMemo(() => singleTabStop ? { getActiveKey: () => activeKeyRef.current, + isInScope: key => inScopeRef.current.has(key), subscribe: listener => { listeners.current.add(listener); return () => listeners.current.delete(listener); }, register: (key, element) => { const previous = managedByKey.current.get(key); - if (previous) keyByManaged.current.delete(previous); + if (previous) { + keyByManaged.current.delete(previous); + resizeObserverRef.current?.unobserve(previous); + } if (element) { managedByKey.current.set(key, element); keyByManaged.current.set(element, key); + resizeObserverRef.current?.observe(element); } else { + // Scope is kept: a ref that changes identity unregisters and registers again in + // one commit, and reconcile prunes keys that stay unregistered. managedByKey.current.delete(key); } // A tool that stops being managed (unmounted, or given its own tab index) may have - // been the active one; a new tool may be the first available one. + // been the active one; a new tool may be the first available one or lie out of scope. scheduleReconcile(); }, - activate: key => setActiveKey(key), + // A tool outside the toolbar's scope cannot hold its single Tab stop. + activate: key => { + if (inScopeRef.current.has(key)) setActiveKey(key); + }, } : null, [singleTabStop, scheduleReconcile, setActiveKey]); const onKeyDown = (event: KeyboardEvent) => { diff --git a/Source/Toolbar/Toolbar.stories.tsx b/Source/Toolbar/Toolbar.stories.tsx index 198f936e..57c2aba4 100644 --- a/Source/Toolbar/Toolbar.stories.tsx +++ b/Source/Toolbar/Toolbar.stories.tsx @@ -304,7 +304,6 @@ export const WithEditableTool: Story = { ), }; -/** Demonstrates the active (selected) state of a toolbar button. */ /** One Tab stop for the toolbar's own tools; the input keeps its own Tab stop. */ export const SingleTabStop: Story = { render: () => ( @@ -335,6 +334,7 @@ export const SingleTabStop: Story = { }, }; +/** Demonstrates the active (selected) state of a toolbar button. */ export const WithActiveButton: Story = { render: () => { const ActiveDemo = () => { diff --git a/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx b/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx new file mode 100644 index 00000000..5b5d52b7 --- /dev/null +++ b/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx @@ -0,0 +1,165 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// @vitest-environment jsdom + +import { act } from 'react'; +import { createPortal } from 'react-dom'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, it, vi } from 'vitest'; +import { expect } from 'chai'; +import { Toolbar } from '../Toolbar'; +import { ToolbarButton } from '../ToolbarButton'; +import { ToolbarFanOutItem } from '../ToolbarFanOutItem'; +import { ToolbarFolder } from '../ToolbarFolder'; +import { ToolbarFocusMode } from '../../Common/ToolbarFocusMode'; + +let container: HTMLDivElement; +let portalTarget: HTMLDivElement; +let root: Root; +const render = async (element: React.ReactNode) => { + await act(async () => root.render(element)); + // Registrations schedule one check per microtask. + await act(async () => { await Promise.resolve(); }); +}; +const tabIndexOf = (label: string) => document.querySelector(`[aria-label="${label}"]`)!.getAttribute('tabindex'); + +beforeEach(() => { + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement('div'); + portalTarget = document.createElement('div'); + document.body.append(container, portalTarget); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + portalTarget.remove(); +}); + +describe('when a tool is rendered through a portal', () => { + it('should keep the portaled tool its native Tab stop and ignore its focus', async () => { + await render( + + + + {createPortal(, portalTarget)} + , + ); + expect(portalTarget.querySelector('[aria-label="Settings"]')!.getAttribute('tabindex')).to.not.equal('-1'); + await act(async () => portalTarget.querySelector('[aria-label="Settings"]')!.focus()); + expect(tabIndexOf('Draw')).to.equal('0'); + expect(tabIndexOf('Erase')).to.equal('-1'); + }); +}); + +describe('when a tool sits under a nested consumer toolbar', () => { + it('should keep the nested tool its native Tab stop', async () => { + await render( + + +
+ +
+ , + ); + expect(tabIndexOf('Draw')).to.equal('0'); + expect(tabIndexOf('Settings')).to.not.equal('-1'); + }); +}); + +describe('when a nested toolbar uses the default mode', () => { + it('should keep the inner tools their own Tab stops', async () => { + await render( + + + + + + + + , + ); + expect([tabIndexOf('Draw'), tabIndexOf('Erase')]).to.deep.equal(['0', '-1']); + expect([tabIndexOf('Circle'), tabIndexOf('Square')]).to.deep.equal(['0', '0']); + }); +}); + +describe('when a toolbar has a fan-out item', () => { + it('should make the fan-out trigger part of the single Tab stop', async () => { + await render( + + + + + + , + ); + expect([tabIndexOf('Draw'), tabIndexOf('Layouts')]).to.deep.equal(['0', '-1']); + await act(async () => document.querySelector('[aria-label="Layouts"]')!.focus()); + expect([tabIndexOf('Draw'), tabIndexOf('Layouts')]).to.deep.equal(['-1', '0']); + }); +}); + +describe('when a folder trigger has an explicit tab index', () => { + it('should keep that tab index and leave the other tools in the single Tab stop', async () => { + await render( + + + + + + + , + ); + expect([tabIndexOf('Draw'), tabIndexOf('Erase'), tabIndexOf('Shapes')]).to.deep.equal(['0', '-1', '0']); + }); +}); + +describe('when switching from the default mode to a single Tab stop', () => { + it('should give only the first tool the Tab stop', async () => { + const tools = (focusMode: ToolbarFocusMode) => ( + + + + + ); + await render(tools(ToolbarFocusMode.Arrows)); + await render(tools(ToolbarFocusMode.SingleTabStop)); + expect([tabIndexOf('Draw'), tabIndexOf('Erase')]).to.deep.equal(['0', '-1']); + }); +}); + +describe('when a stylesheet alone hides the active tool', () => { + it('should move the Tab stop once the tool is resized away', async () => { + const resized: Array<() => void> = []; + vi.stubGlobal('ResizeObserver', class { + constructor(callback: () => void) { resized.push(callback); } + observe() { /* The spec triggers resizes itself. */ } + unobserve() { /* Nothing to release. */ } + disconnect() { /* Nothing to release. */ } + }); + const style = document.createElement('style'); + style.textContent = '.narrow [aria-label="Draw"] { display: none; }'; + document.head.append(style); + try { + await render( + + + + , + ); + expect(tabIndexOf('Draw')).to.equal('0'); + document.body.classList.add('narrow'); + await act(async () => { + resized.forEach(callback => callback()); + await Promise.resolve(); + }); + expect(tabIndexOf('Erase')).to.equal('0'); + } finally { + document.body.classList.remove('narrow'); + style.remove(); + vi.unstubAllGlobals(); + } + }); +}); From bf50a8bfab9616714f9f42c99a768a0e351cbbd7 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 08:10:55 +0200 Subject: [PATCH 7/9] Use vi.waitFor in the dismissal specs instead of an undeclared package --- .../for_CommandDialog/when_dismissal_is_configured.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts b/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts index fb160dbe..dbfafc8a 100644 --- a/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts +++ b/Source/CommandDialog/for_CommandDialog/when_dismissal_is_configured.ts @@ -7,7 +7,6 @@ import React from 'react'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { vi } from 'vitest'; -import { waitFor } from '@testing-library/dom'; import { CommandDialog } from '../CommandDialog'; import { CratisComponentsProvider } from '../../Common/CratisComponentsProvider'; @@ -83,7 +82,7 @@ describe('when dismissal is configured on a command dialog', () => { ); }); // Wait for the dialog itself rather than for a fixed delay. - await waitFor(() => { + await vi.waitFor(() => { if (!document.querySelector('[role="dialog"]')) throw new Error('The dialog has not opened yet.'); }); }; From 1cdb20b66b2b147f16f1136c7003c6ce76664a94 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 08:55:21 +0200 Subject: [PATCH 8/9] Record the 4.21.0 API additions in the API surface snapshot --- Source/api-surface.json | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Source/api-surface.json b/Source/api-surface.json index 768a6e53..f407eadd 100644 --- a/Source/api-surface.json +++ b/Source/api-surface.json @@ -121,7 +121,7 @@ "ChatComposerLabels": "export interface ChatComposerLabels { placeholder?: string; insertEmoji?: string; send?: string; sendMessage?: string; reactionPicker?: ReactionPickerLabels; }", "ChatConversation": "ChatConversation: (props: ChatConversationProps) => import(\"react\").JSX.Element", "ChatConversationLabels": "export interface ChatConversationLabels { loading?: string; failed?: string; unauthorized?: string; empty?: string; typing?: string; typingTwo?: string; typingSeveral?: string; quickReplyTo?: string; timestamps?: RelativeTimestampLabels; composer?: ChatComposerLabels; }", - "ChatConversationProps": "export interface ChatConversationProps { messages: TMessage[]; status?: ChatStatus; onSendMessage: (body: string, mentions: ChatMention[]) => void; authorOf?: (authorId: ChatIdentifier) => ChatAuthor; renderAvatar?: (authorId: ChatIdentifier, author: ChatAuthor) => ReactNode; renderAuthorName?: (authorId: ChatIdentifier, author: ChatAuthor) => ReactNode; actions?: ChatMessageAction[]; mentionCandidates?: MentionCandidate[]; resolveMentionCandidates?: (query: string) => MentionCandidate[] | Promise; typingAuthors?: ChatTypingAuthor[]; quickReply?: boolean; buildAvatarUrl?: (params: BuildAvatarUrlParams) => string; autoFocus?: boolean; labels?: ChatConversationLabels; className?: string; }", + "ChatConversationProps": "export interface ChatConversationProps { messages: TMessage[]; status?: ChatStatus; onSendMessage: (body: string, mentions: ChatMention[]) => void; authorOf?: (authorId: ChatIdentifier) => ChatAuthor; renderAvatar?: (authorId: ChatIdentifier, author: ChatAuthor) => ReactNode; renderAuthorName?: (authorId: ChatIdentifier, author: ChatAuthor) => ReactNode; renderMessageExtra?: (message: TMessage) => ReactNode; actions?: ChatMessageAction[]; mentionCandidates?: MentionCandidate[]; resolveMentionCandidates?: (query: string) => MentionCandidate[] | Promise; typingAuthors?: ChatTypingAuthor[]; quickReply?: boolean; buildAvatarUrl?: (params: BuildAvatarUrlParams) => string; autoFocus?: boolean; labels?: ChatConversationLabels; className?: string; }", "ChatIdentifier": "export type ChatIdentifier = string | { toString(): string; };", "ChatMention": "export interface ChatMention { id: ChatIdentifier; name: string; kind: ChatAuthorKind; }", "ChatMessage": "export interface ChatMessage { id: ChatIdentifier; topicId: ChatIdentifier; authorId: ChatIdentifier; body: string; timestamp: Date; mentions?: ChatMention[]; metadata?: Record; }", @@ -133,7 +133,7 @@ "ChatSidebarForObservableQueriesProps": "export interface ChatSidebarForObservableQueriesProps, TMessagesQuery extends IObservableQueryFor, TTopicsArguments extends object = object, TMessagesArguments extends object = object> extends Omit, 'topics' | 'messages' | 'selectedTopicId' | 'topicsStatus' | 'messagesStatus'> { topicsQuery: Constructor; topicsArguments?: TTopicsArguments; messagesQuery: Constructor; messagesArguments: (topicId: ChatIdentifier | undefined) => TMessagesArguments | undefined; }", "ChatSidebarLabels": "export interface ChatSidebarLabels { topics?: string; unnamedTopic?: string; back?: string; close?: string; topicList?: ChatTopicListLabels; conversation?: ChatConversationLabels; }", "ChatSidebarParts": "export interface ChatSidebarParts { backdrop?: ChatSidebarPartAttributes; root?: ChatSidebarPartAttributes; header?: ChatSidebarPartAttributes; title?: ChatSidebarPartAttributes; back?: ChatSidebarButtonAttributes; close?: ChatSidebarButtonAttributes; content?: ChatSidebarPartAttributes; }", - "ChatSidebarProps": "export interface ChatSidebarProps extends Omit, 'messages' | 'onSendMessage' | 'labels' | 'className' | 'status'> { open: boolean; onClose: () => void; topics: TTopic[]; messages: TMessage[]; topicActions?: ChatTopicAction[]; topicsStatus?: ChatStatus; messagesStatus?: ChatStatus; selectedTopicId?: ChatIdentifier | null; onTopicSelected?: (topicId: ChatIdentifier | undefined, topic?: TTopic) => void; onStartTopic?: () => ChatIdentifier | undefined | Promise; onSendMessage: (topicId: ChatIdentifier, body: string, mentions: ChatMention[]) => void; onRequestTopicName?: (topic: TTopic, firstMessageBody: string) => void; isTopicUnnamed?: (topic: TTopic) => boolean; position?: 'left' | 'right'; width?: string; modal?: boolean; labels?: ChatSidebarLabels; className?: string; pt?: ChatSidebarParts; }", + "ChatSidebarProps": "export interface ChatSidebarProps extends Omit, 'messages' | 'onSendMessage' | 'labels' | 'className' | 'status'> { renderHeaderActions?: (openTopic: TTopic | undefined) => ReactNode; open: boolean; onClose: () => void; topics: TTopic[]; messages: TMessage[]; topicActions?: ChatTopicAction[]; topicsStatus?: ChatStatus; messagesStatus?: ChatStatus; selectedTopicId?: ChatIdentifier | null; onTopicSelected?: (topicId: ChatIdentifier | undefined, topic?: TTopic) => void; onStartTopic?: () => ChatIdentifier | undefined | Promise; onSendMessage: (topicId: ChatIdentifier, body: string, mentions: ChatMention[]) => void; onRequestTopicName?: (topic: TTopic, firstMessageBody: string) => void; isTopicUnnamed?: (topic: TTopic) => boolean; position?: 'left' | 'right'; width?: string; modal?: boolean; labels?: ChatSidebarLabels; className?: string; pt?: ChatSidebarParts; }", "ChatStatus": "export declare enum ChatStatus { Ready = \"ready\", Loading = \"loading\", Failed = \"failed\", Unauthorized = \"unauthorized\" }", "ChatTopic": "export interface ChatTopic { id: ChatIdentifier; name?: string; startedBy?: ChatIdentifier; started?: Date; lastActivity?: Date; metadata?: Record; }", "ChatTopicAction": "export interface ChatTopicAction { id: string; label: string; icon?: string | ReactNode; isAvailable?: (topic: TTopic) => boolean; onInvoke: (topic: TTopic) => void; }", @@ -361,7 +361,7 @@ "ToggleGroupPartAttributes": "export type ToggleGroupPartAttributes = Pick, 'className' | 'style' | 'title'> & { [dataAttribute: `data-${string}`]: string | number | boolean | undefined; };", "ToggleGroupParts": "export interface ToggleGroupParts { root?: ToggleGroupPartAttributes; option?: ToggleGroupPartAttributes; icon?: ToggleGroupPartAttributes; label?: ToggleGroupPartAttributes; }", "ToggleGroupProps": "export interface ToggleGroupProps { options: ToggleGroupOption[]; value: string; onChange: (value: string) => void; disabled?: boolean; 'aria-labelledby'?: string; 'aria-label'?: string; 'aria-describedby'?: string; invalid?: boolean; className?: string; pt?: ToggleGroupParts; }", - "ToolbarFocusMode": "export declare enum ToolbarFocusMode { Arrows = \"arrows\", None = \"none\" }", + "ToolbarFocusMode": "export declare enum ToolbarFocusMode { Arrows = \"arrows\", SingleTabStop = \"singleTabStop\", None = \"none\" }", "Tooltip": "Tooltip: (props: TooltipProps) => ReactElement>", "TooltipPosition": "export type TooltipPosition = 'top' | 'right' | 'bottom' | 'left';", "TooltipProps": "export interface TooltipProps { content?: ReactNode; position?: TooltipPosition; disabled?: boolean; className?: string; children: ReactElement; }", @@ -395,7 +395,7 @@ "ColumnFilterOption": "export interface ColumnFilterOption { label: string; value: unknown; }", "ColumnProps": "export interface ColumnProps { field?: string; header?: React.ReactNode; body?: (rowData: TData) => React.ReactNode; sortable?: boolean; filter?: boolean; filterField?: string; filterPlaceholder?: string; dataType?: ColumnFilterDataType; showFilterMatchModes?: boolean; filterElement?: ColumnFilterElement; filterOptions?: ColumnFilterOption[]; filterLabels?: Partial; filterPt?: ColumnFilterMenuParts; selectionMode?: 'single' | 'multiple'; style?: React.CSSProperties; className?: string; headerStyle?: React.CSSProperties; headerClassName?: string; bodyStyle?: React.CSSProperties; bodyClassName?: string; }", "DataTableCore": "DataTableCore: (props: DataTableCoreProps) => React.JSX.Element", - "DataTableCoreProps": "export interface DataTableCoreProps { data: TData[]; children?: ReactNode; dataKey?: string; emptyMessage: ReactNode; status?: DataTableStatus; loadingMessage?: ReactNode; failureMessage?: ReactNode; unauthorizedMessage?: ReactNode; selectionMode?: 'single' | 'multiple'; selectionAriaLabel?: string; selectAllAriaLabel?: string; selection?: TData | null; onSelectionChange?: (event: DataTableSelectionChangeEvent) => void; selectedItems?: TData[]; onSelectedItemsChange?: (items: TData[]) => void; onRowClick?: (event: DataTableRowClickEvent) => void; rowClassName?: (rowData: TData) => string; globalFilterFields?: string[]; globalSearchPlaceholder?: string; globalSearchAriaLabel?: string; defaultFilters?: DataTableFilterMeta; onFilter?: (filters: DataTableFilterMeta) => void; scrollable?: boolean; scrollHeight?: string; className?: string; style?: CSSProperties; pt?: DataTableParts; ptOptions?: object; unstyled?: boolean; }", + "DataTableCoreProps": "export interface DataTableCoreProps { data: TData[]; children?: ReactNode; dataKey?: string; emptyMessage: ReactNode; status?: DataTableStatus; loadingMessage?: ReactNode; failureMessage?: ReactNode; unauthorizedMessage?: ReactNode; selectionMode?: 'single' | 'multiple'; selectionAriaLabel?: string; selectAllAriaLabel?: string; selection?: TData | null; onSelectionChange?: (event: DataTableSelectionChangeEvent) => void; selectedItems?: TData[]; onSelectedItemsChange?: (items: TData[]) => void; onRowClick?: (event: DataTableRowClickEvent) => void; rowClassName?: (rowData: TData) => string; globalFilterFields?: string[]; globalSearchPlaceholder?: string; globalSearchAriaLabel?: string; defaultFilters?: DataTableFilterMeta; filters?: DataTableFilterMeta; onFilter?: (filters: DataTableFilterMeta) => void; globalFilter?: string; onGlobalFilterChange?: (globalFilter: string) => void; sort?: DataTableSort | null; onSortChange?: (sort: DataTableSort | null) => void; rowProcessing?: DataTableRowProcessing; scrollable?: boolean; scrollHeight?: string; className?: string; style?: CSSProperties; pt?: DataTableParts; ptOptions?: object; unstyled?: boolean; }", "DataTableCustomFilterMatchMode": "export type DataTableCustomFilterMatchMode = string & { readonly [customFilterMatchMode]: true; };", "DataTableFilterConstraint": "export interface DataTableFilterConstraint { value: unknown; matchMode?: DataTableFilterMatchMode; }", "DataTableFilterEntry": "export type DataTableFilterEntry = DataTableFilterConstraint | DataTableOperatorFilterConstraint;", @@ -410,7 +410,10 @@ "DataTableOperatorFilterConstraint": "export interface DataTableOperatorFilterConstraint { operator?: string; constraints: DataTableFilterConstraint[]; }", "DataTableParts": "export interface DataTableParts { root?: HTMLAttributes; search?: HTMLAttributes; searchInput?: React.InputHTMLAttributes; tableContainer?: HTMLAttributes; table?: TableHTMLAttributes; head?: HTMLAttributes; headerRow?: HTMLAttributes; headerCell?: ThHTMLAttributes; body?: HTMLAttributes; row?: HTMLAttributes; cell?: TdHTMLAttributes; emptyRow?: HTMLAttributes; emptyCell?: TdHTMLAttributes; loadingRow?: HTMLAttributes; loadingCell?: TdHTMLAttributes; failureRow?: HTMLAttributes; failureCell?: TdHTMLAttributes; }", "DataTableRowClickEvent": "export interface DataTableRowClickEvent { data: TData; index: number; }", + "DataTableRowProcessing": "export declare enum DataTableRowProcessing { Loaded = \"loaded\", None = \"none\" }", "DataTableSelectionChangeEvent": "export interface DataTableSelectionChangeEvent { value: TData | null; originalEvent?: SyntheticEvent; }", + "DataTableSort": "export interface DataTableSort { field: string; direction: DataTableSortDirection; }", + "DataTableSortDirection": "export declare enum DataTableSortDirection { Ascending = \"ascending\", Descending = \"descending\" }", "DataTableStatus": "export declare enum DataTableStatus { Ready = \"ready\", Loading = \"loading\", Failed = \"failed\", Unauthorized = \"unauthorized\" }", "TablePaginator": "TablePaginator: (props: TablePaginatorProps) => import(\"react\").ReactElement>", "TablePaginatorParts": "export interface TablePaginatorParts { root?: HTMLAttributes; range?: HTMLAttributes; info?: HTMLAttributes; first?: ButtonParts; previous?: ButtonParts; next?: ButtonParts; last?: ButtonParts; }", @@ -558,7 +561,7 @@ "ToolbarFanOutItem": "ToolbarFanOutItem: (props: ToolbarFanOutItemProps) => React.JSX.Element", "ToolbarFanOutItemProps": "export interface ToolbarFanOutItemProps { icon: Icon; tooltip: string; tooltipPosition?: TooltipPosition; fanOutDirection?: 'right' | 'left' | 'up' | 'down'; className?: string; pt?: ToolbarFanOutParts; children: ReactNode; }", "ToolbarFanOutParts": "export interface ToolbarFanOutParts { root?: HTMLAttributes; trigger?: ButtonHTMLAttributes; panel?: HTMLAttributes; }", - "ToolbarFocusMode": "export declare enum ToolbarFocusMode { Arrows = \"arrows\", None = \"none\" }", + "ToolbarFocusMode": "export declare enum ToolbarFocusMode { Arrows = \"arrows\", SingleTabStop = \"singleTabStop\", None = \"none\" }", "ToolbarFolder": "ToolbarFolder: (props: ToolbarFolderProps) => import(\"react\").JSX.Element", "ToolbarFolderMode": "export type ToolbarFolderMode = 'grid' | 'list';", "ToolbarFolderParts": "export interface ToolbarFolderParts { root?: HTMLAttributes; trigger?: ButtonHTMLAttributes; panel?: HTMLAttributes; }", From dc8dc264d071eef32a3067cfc0e36a016c9df0c5 Mon Sep 17 00:00:00 2001 From: woksin Date: Tue, 29 Sep 2026 09:09:25 +0200 Subject: [PATCH 9/9] Skip re-registering a tool whose tooltip rebuilt its ref, and cover the documented contracts A tooltip wrapper builds a new merged ref each render, so React detaches and re-attaches the same element. Removal now waits for a microtask and a re-attach of the same element costs nothing. New specs cover ActionMenubar in SingleTabStop mode and DataTableCore's search and column filter callbacks through the UI in controlled mode. --- .../when_using_a_single_tab_stop.tsx | 59 +++++++++++++++++++ Source/Common/useToolbarKeyboardNavigation.ts | 51 ++++++++++++---- ...when_controlling_sort_and_filter_state.tsx | 50 +++++++++++++++- ..._sit_at_the_edges_of_a_single_tab_stop.tsx | 36 ++++++++++- 4 files changed, 181 insertions(+), 15 deletions(-) create mode 100644 Source/Common/for_ActionMenubar/when_using_a_single_tab_stop.tsx diff --git a/Source/Common/for_ActionMenubar/when_using_a_single_tab_stop.tsx b/Source/Common/for_ActionMenubar/when_using_a_single_tab_stop.tsx new file mode 100644 index 00000000..2d616f7e --- /dev/null +++ b/Source/Common/for_ActionMenubar/when_using_a_single_tab_stop.tsx @@ -0,0 +1,59 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +// @vitest-environment jsdom + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, it } from 'vitest'; +import { expect } from 'chai'; +import { ActionMenubar, type ActionMenuItem } from '../ActionMenubar'; +import { ToolbarFocusMode } from '../ToolbarFocusMode'; + +let container: HTMLDivElement; +let root: Root; +const render = async (model: ActionMenuItem[], pt?: React.ComponentProps['pt']) => { + await act(async () => root.render()); + await act(async () => { await Promise.resolve(); }); +}; +const action = (label: string) => + Array.from(container.querySelectorAll('button')).find(button => button.textContent?.includes(label))!; +const tabIndexes = (...labels: string[]) => labels.map(label => action(label).getAttribute('tabindex')); + +beforeEach(() => { + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement('div'); + document.body.append(container); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); +}); + +describe('when an action menubar uses a single Tab stop', () => { + it('should move the Tab stop to a focused action and call the consumer focus handler once', async () => { + let focused = 0; + await render([{ label: 'Save' }, { label: 'Share' }], { root: { onFocus: () => { focused++; } } }); + expect(tabIndexes('Save', 'Share')).to.deep.equal(['0', '-1']); + await act(async () => action('Share').focus()); + expect(focused).to.equal(1); + expect(tabIndexes('Save', 'Share')).to.deep.equal(['-1', '0']); + }); + + it('should give every action its explicit tab index and no single Tab stop', async () => { + await render([{ label: 'Save' }, { label: 'Share' }], { root: { tabIndex: 0 } }); + expect(tabIndexes('Save', 'Share')).to.deep.equal(['0', '0']); + }); + + it('should skip a disabled first action', async () => { + await render([{ label: 'Save', disabled: true }, { label: 'Share' }, { label: 'Delete' }]); + expect(tabIndexes('Share', 'Delete')).to.deep.equal(['0', '-1']); + }); + + it('should leave the tab index of template content alone', async () => { + await render([{ label: 'Save' }, { label: 'Search', template: () => }]); + expect(container.querySelector('input')!.hasAttribute('tabindex')).to.equal(false); + expect(tabIndexes('Save')).to.deep.equal(['0']); + }); +}); diff --git a/Source/Common/useToolbarKeyboardNavigation.ts b/Source/Common/useToolbarKeyboardNavigation.ts index d37820a3..2e9da9e8 100644 --- a/Source/Common/useToolbarKeyboardNavigation.ts +++ b/Source/Common/useToolbarKeyboardNavigation.ts @@ -96,6 +96,29 @@ export const useToolbarKeyboardNavigation = (orientation: Orientation, focusMode }); }, [reconcile]); + // Tools that unregistered and did not register again: forget them, and choose a new Tab stop + // if one of them was active (unmounted, or given its own tab index). + const pendingRemovalRef = useRef(new Set()); + const removalCheckScheduledRef = useRef(false); + const scheduleRemovalCheck = useCallback(() => { + if (removalCheckScheduledRef.current) return; + removalCheckScheduledRef.current = true; + queueMicrotask(() => { + removalCheckScheduledRef.current = false; + if (pendingRemovalRef.current.size === 0) return; + for (const key of pendingRemovalRef.current) { + const element = managedByKey.current.get(key); + if (element) { + keyByManaged.current.delete(element); + resizeObserverRef.current?.unobserve(element); + } + managedByKey.current.delete(key); + } + pendingRemovalRef.current.clear(); + reconcile(); + }); + }, [reconcile]); + useLayoutEffect(reconcile); useLayoutEffect(() => { const root = rootRef.current; @@ -139,28 +162,32 @@ export const useToolbarKeyboardNavigation = (orientation: Orientation, focusMode }, register: (key, element) => { const previous = managedByKey.current.get(key); + if (!element) { + // A tooltip wrapper builds a new merged ref on every render, so React detaches and + // re-attaches the same element within one commit. Removal waits for a microtask, + // and a re-attach of the same element in between costs nothing. + if (previous) pendingRemovalRef.current.add(key); + scheduleRemovalCheck(); + return; + } + if (previous === element && pendingRemovalRef.current.delete(key)) return; + pendingRemovalRef.current.delete(key); + if (previous === element) return; if (previous) { keyByManaged.current.delete(previous); resizeObserverRef.current?.unobserve(previous); } - if (element) { - managedByKey.current.set(key, element); - keyByManaged.current.set(element, key); - resizeObserverRef.current?.observe(element); - } else { - // Scope is kept: a ref that changes identity unregisters and registers again in - // one commit, and reconcile prunes keys that stay unregistered. - managedByKey.current.delete(key); - } - // A tool that stops being managed (unmounted, or given its own tab index) may have - // been the active one; a new tool may be the first available one or lie out of scope. + managedByKey.current.set(key, element); + keyByManaged.current.set(element, key); + resizeObserverRef.current?.observe(element); + // A new tool may be the first available one or lie out of scope. scheduleReconcile(); }, // A tool outside the toolbar's scope cannot hold its single Tab stop. activate: key => { if (inScopeRef.current.has(key)) setActiveKey(key); }, - } : null, [singleTabStop, scheduleReconcile, setActiveKey]); + } : null, [singleTabStop, scheduleReconcile, scheduleRemovalCheck, setActiveKey]); const onKeyDown = (event: KeyboardEvent) => { if (focusMode === ToolbarFocusMode.None || event.defaultPrevented || event.isPropagationStopped() || diff --git a/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx b/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx index 3392baf4..5709508b 100644 --- a/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx +++ b/Source/DataTables/for_DataTableCore/when_controlling_sort_and_filter_state.tsx @@ -12,7 +12,7 @@ import { DataTableCore, type DataTableCoreProps } from '../DataTableCore'; import type { DataTableSort } from '../DataTableSort'; import { DataTableSortDirection } from '../DataTableSortDirection'; import { DataTableRowProcessing } from '../DataTableRowProcessing'; -import { DataTableFilterMatchMode } from '../DataTableFilterMeta'; +import { DataTableFilterMatchMode, type DataTableFilterMeta } from '../DataTableFilterMeta'; interface Product { id: number; @@ -32,11 +32,22 @@ const render = async (props: Partial>) => { await act(async () => { root.render( data={data} dataKey='id' emptyMessage='None' globalFilterFields={['name']} {...props}> - field='name' header='Name' sortable /> + field='name' header='Name' sortable filter /> , ); }); }; +const typeInto = (input: HTMLInputElement, text: string) => { + Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(input, text); + input.dispatchEvent(new Event('input', { bubbles: true })); +}; +const buttonLabeled = (label: string) => + Array.from(document.querySelectorAll('[data-cratis-part="filter-actions"] button')) + .find(button => button.textContent?.trim() === label)!; +const openFilterMenu = async () => { + if (document.querySelector('[data-cratis-part="filter-actions"]')) return; + await act(async () => container.querySelector('[data-cratis-part="filter-trigger"]')!.click()); +}; const clickSort = async () => { await act(async () => container.querySelector('[data-cratis-part="sort"]')!.click()); }; @@ -94,4 +105,39 @@ describe('when controlling sort and filter state', () => { }); expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); }); + + it('should report typed search text without changing the controlled search text', async () => { + const reported: string[] = []; + await render({ globalFilter: 'bra', onGlobalFilterChange: text => reported.push(text) }); + const input = container.querySelector('[data-cratis-part="search-input"]')!; + await act(async () => typeInto(input, 'char')); + expect(reported).to.deep.equal(['char']); + expect(input.value).to.equal('bra'); + expect(names()).to.deep.equal(['Bravo']); + }); + + it('should report applied and cleared column filters without changing the controlled filters', async () => { + const reported: DataTableFilterMeta[] = []; + await render({ filters: {}, onFilter: filters => reported.push(filters) }); + await openFilterMenu(); + await act(async () => typeInto(document.querySelector('[data-cratis-part="filter-menu"] input')!, 'Alpha')); + await act(async () => buttonLabeled('Apply').click()); + expect(reported).to.have.length(1); + expect(reported[0].name).to.deep.include({ value: 'Alpha' }); + expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); + await openFilterMenu(); + await act(async () => buttonLabeled('Clear').click()); + expect(reported).to.have.length(2); + expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); + }); + + it('should report an applied column filter and still render rows as given when row processing is off', async () => { + const reported: DataTableFilterMeta[] = []; + await render({ rowProcessing: DataTableRowProcessing.None, onFilter: filters => reported.push(filters) }); + await openFilterMenu(); + await act(async () => typeInto(document.querySelector('[data-cratis-part="filter-menu"] input')!, 'Alpha')); + await act(async () => buttonLabeled('Apply').click()); + expect(reported).to.have.length(1); + expect(names()).to.deep.equal(['Charlie', 'Alpha', 'Bravo']); + }); }); diff --git a/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx b/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx index 5b5d52b7..910c5359 100644 --- a/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx +++ b/Source/Toolbar/for_Toolbar/when_tools_sit_at_the_edges_of_a_single_tab_stop.tsx @@ -3,7 +3,7 @@ // @vitest-environment jsdom -import { act } from 'react'; +import { act, useState } from 'react'; import { createPortal } from 'react-dom'; import { createRoot, type Root } from 'react-dom/client'; import { afterEach, beforeEach, describe, it, vi } from 'vitest'; @@ -163,3 +163,37 @@ describe('when a stylesheet alone hides the active tool', () => { } }); }); + +describe('when a tool re-renders inside its tooltip', () => { + it('should not register the same element again', async () => { + let observed = 0; + vi.stubGlobal('ResizeObserver', class { + observe() { observed++; } + unobserve() { /* Nothing to release. */ } + disconnect() { /* Nothing to release. */ } + }); + let rerender: () => void = () => undefined; + const Tools = () => { + const [count, setCount] = useState(0); + rerender = () => setCount(count + 1); + return ( + + + + + ); + }; + try { + await render(); + const initial = observed; + for (let index = 0; index < 3; index++) { + await act(async () => rerender()); + await act(async () => { await Promise.resolve(); }); + } + expect(observed).to.equal(initial); + expect([tabIndexOf('Draw 3'), tabIndexOf('Erase')]).to.deep.equal(['0', '-1']); + } finally { + vi.unstubAllGlobals(); + } + }); +});