diff --git a/.changeset/board-selection-cancel-capture.md b/.changeset/board-selection-cancel-capture.md new file mode 100644 index 000000000..18c7b6238 --- /dev/null +++ b/.changeset/board-selection-cancel-capture.md @@ -0,0 +1,5 @@ +--- +'@cube-dev/ui-kit': patch +--- + +**`Board`: pressing an interactive control inside a widget now always drops the selection.** `selectionCancel` promises that a press on an interactive descendant drops the selection, but the reset used to ride on the widget host's bubble-phase `onPointerDown` — so it only happened for presses that actually reached the host. A control that calls `stopPropagation()` first (React Aria's `usePress` does by default, and charting libraries do it on the native event) or one that renders in a portal never got there, which is why a widget's gear button dropped the selection while the chart's own toolbar button silently left it standing. The reset now runs in the capture phase, both through React (so a portal declared inside the widget is still covered) and as a native listener on the host node itself (so a descendant that stops the native event cannot pre-empt it). Nothing is stopped or prevented in either handler, so every control keeps its press, its focus and its default behaviour. A press on content a widget portaled outside the board also no longer starts a marquee behind the overlay. diff --git a/src/components/layout/Board/Board.docs.mdx b/src/components/layout/Board/Board.docs.mdx index 2edf21e6d..a981d60b3 100644 --- a/src/components/layout/Board/Board.docs.mdx +++ b/src/components/layout/Board/Board.docs.mdx @@ -64,7 +64,7 @@ Dragging and resizing are powered by React Aria's `useMove`, so they work with m - **`selectedKeys`** `string[]` — Controlled selection. Keys are layout item ids (`LayoutItem.i`). - **`defaultSelectedKeys`** `string[]` — Initial selection for uncontrolled usage. - **`onSelectionChange`** `(keys: string[]) => void` — Called when the selection changes. Keys are deduped and returned in the board's layout order, never in click order. -- **`selectionCancel`** `string` (default: `BOARD_SELECTION_CANCEL`) — CSS selector marking interactive descendants. A press on one never selects and never starts a drag, so the control keeps its own click _and_ its native focus; it also drops the selection, since interacting with a widget's content means the user has moved on. On a selectable board this doubles as the drag guard, so form controls stay usable without also configuring `dragCancel`. The default covers native form controls, links, and the common ARIA widget roles, plus `[data-no-select]` as an escape hatch. Pass `''` to disable the guard. Can be overridden per widget. +- **`selectionCancel`** `string` (default: `BOARD_SELECTION_CANCEL`) — CSS selector marking interactive descendants. A press on one never selects and never starts a drag, so the control keeps its own click _and_ its native focus; it also drops the selection, since interacting with a widget's content means the user has moved on — and that holds however the control handles the press, including one that stops propagation or renders in a portal (a menu opened from a widget's toolbar). On a selectable board this doubles as the drag guard, so form controls stay usable without also configuring `dragCancel`. The default covers native form controls, links, and the common ARIA widget roles, plus `[data-no-select]` as an escape hatch. Pass `''` to disable the guard. Can be overridden per widget. - **`allowMarqueeSelection`** `boolean` (default: `selectionMode === 'multiple'`) — Draw a rubber-band selection when a drag starts on empty board space. A press on a widget selects and drags instead, so the lasso owns empty canvas only. Hold Shift or Cmd/Ctrl to add to the existing selection rather than replacing it. A board sized to its content has no empty space left once the grid fills up, which makes the lasso undiscoverable — use `extraRows` to keep a band of it. - **`onWidgetsDelete`** `(keys: string[]) => void` — Called when Delete/Backspace is pressed with a non-empty selection and focus is not in an editable field. **Board never mutates the layout itself** — removing the widgets is yours to do, which is what lets you make it undoable. Board only handles these keys when this handler is set. - **`constraints`** `LayoutConstraint[]` — Grid/item layout constraints. diff --git a/src/components/layout/Board/Board.test.tsx b/src/components/layout/Board/Board.test.tsx index 1f84fda57..09e478ad2 100644 --- a/src/components/layout/Board/Board.test.tsx +++ b/src/components/layout/Board/Board.test.tsx @@ -1,3 +1,6 @@ +import { useEffect, useRef } from 'react'; +import { createPortal } from 'react-dom'; + import { act, fireEvent, @@ -11,6 +14,7 @@ import { Tab, Tabs } from '../../navigation/Tabs'; import { Board } from './index'; +import type { ReactNode } from 'react'; import type { LayoutConstraint, LayoutItem } from './grid-core'; const baseLayout = [ @@ -3214,6 +3218,110 @@ describe('Board', () => { expect(onSelectionChange).toHaveBeenCalledWith([]); }); + // Regression (CUB-3827): the reset used to ride on the host's bubble-phase + // `onPointerDown`, so a control that never let the press through kept the + // selection standing — which is why a widget's gear button dropped it and + // the chart's own toolbar button did not. Three ways a press can miss the + // host, all of which must still drop the selection. + describe('however the press reaches the widget', () => { + const twoWidgetLayout = [ + { i: 'a', x: 0, y: 0, w: 2, h: 1 }, + { i: 'b', x: 2, y: 0, w: 2, h: 1 }, + ]; + + /** Selects `b`, then presses `content`'s control inside widget `a`. */ + const pressInsideA = async (content: ReactNode) => { + const onSelectionChange = vi.fn(); + + render( + + + {content} + + + B + + , + ); + + await userEvent.click(screen.getByTestId('B')); + expect(onSelectionChange).toHaveBeenLastCalledWith(['b']); + onSelectionChange.mockClear(); + + fireEvent.pointerDown(screen.getByRole('button', { name: 'Ctl' }), { + button: 0, + pointerId: 1, + }); + + return onSelectionChange; + }; + + // What React Aria's `usePress` does by default. + it('drops it when the control stops the React press', async () => { + const onSelectionChange = await pressInsideA( + , + ); + + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + + // What a charting or mapping library does with its own listeners: the + // native event never reaches React, so no React handler on the host runs. + it('drops it when the control stops the native press', async () => { + function NativeStopButton() { + const ref = useRef(null); + + useEffect(() => { + const node = ref.current; + if (!node) return; + + const stop = (event: Event) => event.stopPropagation(); + + node.addEventListener('pointerdown', stop); + + return () => node.removeEventListener('pointerdown', stop); + }, []); + + return ( + + ); + } + + const onSelectionChange = await pressInsideA(); + + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + + // A menu opened from a widget renders outside the host's DOM subtree, + // but still inside its React tree — so the React capture handler is the + // only one that can see this press. + it('drops it when a portaled control stops the press', async () => { + const onSelectionChange = await pressInsideA( + createPortal( + , + document.body, + ) as ReactNode, + ); + + expect(onSelectionChange).toHaveBeenCalledWith([]); + }); + }); + it('keeps the selection when pressing a widget inside it', async () => { const onSelectionChange = vi.fn(); const { widget } = renderSelectableBoard({ onSelectionChange }); diff --git a/src/components/layout/Board/Board.tsx b/src/components/layout/Board/Board.tsx index 582cfe986..3ab4929dd 100644 --- a/src/components/layout/Board/Board.tsx +++ b/src/components/layout/Board/Board.tsx @@ -1190,6 +1190,10 @@ function BoardInner( const content = contentRef.current; if (!content) return; + // React events also arrive from portaled content: an overlay a widget opened + // renders outside this DOM subtree but still propagates along the React + // tree, and a press in a popover must never start a lasso behind it. + if (target && !content.contains(target)) return; // Suppress the compatibility mouse events that would begin a native text // selection. Safe here specifically because every case that wants default diff --git a/src/components/layout/Board/WidgetHost.tsx b/src/components/layout/Board/WidgetHost.tsx index f92d5d6ad..fd94fe9da 100644 --- a/src/components/layout/Board/WidgetHost.tsx +++ b/src/components/layout/Board/WidgetHost.tsx @@ -1,5 +1,12 @@ import { Styles, tasty } from '@tenphi/tasty'; -import { CSSProperties, ReactNode, useMemo, useRef, useState } from 'react'; +import { + CSSProperties, + ReactNode, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { useFocusRing, useFocusWithin, useHover, useMove } from 'react-aria'; import { createPortal } from 'react-dom'; @@ -753,6 +760,59 @@ export function WidgetHost(props: WidgetHostProps) { return !!(selectionCancel && el?.closest?.(selectionCancel)); }; + /** + * Pressing something interactive means the user has moved on from the + * selection, so the selection is dropped — always, whatever the control is and + * whatever it does with the event. + * + * That "always" is why this is a *capture*-phase handler and not part of + * `handleSelectPointerDown` below. A bubble-phase handler only sees the + * presses that reach the host, and a control is free to call + * `stopPropagation()` before that happens — React Aria's `usePress` does it by + * default, which is why some in-widget buttons dropped the selection and + * others silently did not. Capture runs top-down, so it lands before any + * descendant can speak. Portaled controls (a menu opened from a widget's + * toolbar) are covered too: React propagates events along the React tree, so a + * portal declared inside this widget still passes through here. + * + * Nothing is stopped or prevented here — the handler only reads the target, so + * the control keeps its press, its focus and its default behaviour intact. + */ + const handleSelectPointerDownCapture = (e: React.PointerEvent) => { + if (!isSelectable || e.button !== 0) return; + + if (isInteractiveTarget(e.target)) { + onSelectionReset?.(); + } + }; + + // The React capture handler above is dispatched from the React root, so a + // descendant that stops the *native* event (charting and mapping libraries + // attach their own listeners and do exactly that) keeps it from ever reaching + // React — capture phase included. A native capture listener on the host node + // itself sits below any such descendant and cannot be pre-empted. The two + // overlap for the ordinary case and that costs nothing: clearing an already + // empty selection is a no-op. + useEffect(() => { + const node = hostRef.current; + + if (!node || !isSelectable || !selectionCancel || !onSelectionReset) return; + + const handleCapture = (event: PointerEvent) => { + if (event.button !== 0) return; + + const target = event.target as HTMLElement | null; + + if (target?.closest?.(selectionCancel)) { + onSelectionReset(); + } + }; + + node.addEventListener('pointerdown', handleCapture, true); + + return () => node.removeEventListener('pointerdown', handleCapture, true); + }, [isSelectable, selectionCancel, onSelectionReset]); + /** * Selecting and starting a drag are the *same* gesture: you grab the thing you * are about to move. So the press selects immediately and the drag arms behind @@ -761,7 +821,8 @@ export function WidgetHost(props: WidgetHostProps) { * modifier. * * - a press on an interactive descendant belongs to that control, so the - * selection is dropped and the press is left alone; + * press is left alone (the selection was already dropped in the capture + * phase above); * - Shift or the platform modifier toggles membership; * - a press on an unselected widget makes it the selection, so the drag that * follows moves exactly what was grabbed; @@ -774,11 +835,7 @@ export function WidgetHost(props: WidgetHostProps) { const handleSelectPointerDown = (e: React.PointerEvent) => { if (!isSelectable || !onSelect || e.button !== 0) return; - if (isInteractiveTarget(e.target)) { - onSelectionReset?.(); - - return; - } + if (isInteractiveTarget(e.target)) return; // A press this widget owns must never reach an ancestor widget host: in a // nested board the outer widget would otherwise select itself on top of the @@ -910,6 +967,7 @@ export function WidgetHost(props: WidgetHostProps) { // its focusability have to come from here instead. const selectionProps = isSelectable ? { + onPointerDownCapture: handleSelectPointerDownCapture, onPointerDown: handleSelectPointerDown, // A draggable widget routes Space through the drag gate below, which // already enforces the host-focused rule; a non-draggable one gets no