diff --git a/.changeset/11546-readonly-canvas-select.md b/.changeset/11546-readonly-canvas-select.md new file mode 100644 index 0000000000..a5c0199732 --- /dev/null +++ b/.changeset/11546-readonly-canvas-select.md @@ -0,0 +1,17 @@ +--- +'@object-ui/app-shell': patch +--- + +On a read-only package, a click on a flow canvas node in Studio Automations selects the node and opens its inspector read-only again (objectui#11546). + +On a read-only canvas the node's press stopped short of claiming the event, so it reached the +canvas background. The background cleared the selection and took the pointer, and the browser +then fired the click at the canvas instead of the node, so the inspector rail kept its empty +state and a packaged flow's node configuration could not be read. + +A press on a node in the designer now belongs to the node on a read-only canvas too. The click +selects the node and the inspector opens with every input disabled, as objectui#11124 set out. +Only the drag is withheld: a press-and-drag on a read-only node moves nothing and writes nothing. +The editable designer drags and selects as before, and the Delete key still deletes only on an +editable canvas. Outside design mode, where a node click selects nothing, a press on a node still +pans the canvas. diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx index 26bfc2bdc1..360d05a34c 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx @@ -7,6 +7,7 @@ import { FlowCanvas } from './FlowCanvas'; import { extractRegions, NODE_H } from './flow-canvas-layout'; import { predictExpandedNodeHeight } from './flow-region-metrics'; import type { FlowProblem } from './flow-problems'; +import { browserClick, browserDrag } from './__tests__/browserClick'; afterEach(cleanup); @@ -499,3 +500,89 @@ describe('FlowCanvas — geometry writes are spec-canonical `position` (#3172)', expectNoLegacyKey(nodes); }); }); + +/** + * objectui#11546 — on a read-only design canvas a node click selects the node; + * only the drag is withheld. Measured in Chromium before the fix: the node's + * pointer-down returned before stopping propagation, the background cleared + * the selection and captured the pointer, and the click landed on the + * viewport. The presses below are routed the way a browser routes them + * (`browserClick` / `browserDrag`), since happy-dom routes nothing by capture. + */ +describe('FlowCanvas — a node press on a read-only design canvas selects, and only the drag is withheld (objectui#11546)', () => { + const NODES = [ + { id: 'a', type: 'start', label: 'Start' }, + { id: 'b', type: 'script', label: 'Do the thing' }, + ]; + const EDGES = [{ source: 'a', target: 'b' }]; + + const renderCanvas = ({ + editable, + designMode, + onSelect = vi.fn(), + onPatch = vi.fn(), + }: { + editable: boolean; + designMode: boolean; + onSelect?: Mock['onSelect']>; + onPatch?: Mock['onPatch']>>; + }) => { + const utils = render( + , + ); + const card = (id: string) => { + const el = utils.container.querySelector(`[data-node-id="${id}"] [role="button"]`) as HTMLElement | null; + expect(el, `the canvas must render node ${id}`).not.toBeNull(); + return el!; + }; + const at = (id: string) => { + const el = utils.container.querySelector(`[data-node-id="${id}"]`) as HTMLElement; + return `${el.style.left},${el.style.top}`; + }; + const viewport = screen.getByRole('application', { name: 'Flow canvas' }); + const panTransform = () => (viewport.firstElementChild as HTMLElement).style.transform; + return { onSelect, onPatch, card, at, viewport, panTransform }; + }; + + it('read-only: a click on a node selects that node and never clears the selection', () => { + const { onSelect, card } = renderCanvas({ editable: false, designMode: true }); + browserClick(card('b')); + expect(onSelect.mock.calls.map(([n]) => n?.id ?? null)).toEqual(['b']); + }); + + it('read-only: a drag on a node moves nothing, writes nothing and pans nothing', () => { + const { onPatch, card, at, panTransform } = renderCanvas({ editable: false, designMode: true }); + const before = { node: at('b'), pan: panTransform() }; + const captor = browserDrag(card('b'), 60, 60); + expect(captor).toBeNull(); + expect(onPatch).not.toHaveBeenCalled(); + expect({ node: at('b'), pan: panTransform() }).toEqual(before); + }); + + it('editable designer (the control): a click selects, and a drag still moves the node', () => { + const { onSelect, onPatch, card } = renderCanvas({ editable: true, designMode: true }); + browserClick(card('b')); + expect(onSelect.mock.calls.map(([n]) => n?.id ?? null)).toEqual(['b']); + + expect(browserDrag(card('b'), 60, 60)).toBe(card('b')); + expect(onPatch).toHaveBeenCalledTimes(1); + const moved = (onPatch.mock.calls[0][0] as { nodes: Array<{ id: string; position?: unknown }> }).nodes.find((n) => n.id === 'b'); + expect(moved?.position).toBeDefined(); + }); + + it('outside design mode a press on a node, which selects nothing, still pans the canvas', () => { + const { onPatch, card, viewport, panTransform } = renderCanvas({ editable: false, designMode: false }); + const before = panTransform(); + expect(browserDrag(card('b'), 60, 60)).toBe(viewport); + expect(panTransform()).not.toBe(before); + expect(onPatch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx index ed387eed00..82501d693e 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx @@ -407,8 +407,17 @@ export function FlowCanvas({ const onNodePointerDown = React.useCallback( (id: string) => (e: React.PointerEvent) => { - if (!editable || e.button !== 0) return; + if (e.button !== 0) return; + // objectui#11546 — a press on a node that answers it (a drag when + // editable, a select in design mode) is the node's, never the + // background's. Reaching `onBgPointerDown` clears the selection and takes + // pointer capture on the viewport, so the browser fires the click at the + // viewport and the node's own select never runs. A read-only design + // canvas withholds only the drag; a press on a node that answers neither + // still pans. + if (!editable && !designMode) return; e.stopPropagation(); + if (!editable) return; const origin = positionOf(id); dragRef.current = { nodeId: id, @@ -420,7 +429,7 @@ export function FlowCanvas({ }; (e.currentTarget as HTMLElement).setPointerCapture?.(e.pointerId); }, - [editable, positionOf], + [designMode, editable, positionOf], ); const onNodePointerMove = React.useCallback( diff --git a/packages/app-shell/src/views/metadata-admin/previews/__tests__/browserClick.ts b/packages/app-shell/src/views/metadata-admin/previews/__tests__/browserClick.ts new file mode 100644 index 0000000000..4fe2e1f4b1 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/__tests__/browserClick.ts @@ -0,0 +1,53 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { fireEvent } from '@testing-library/react'; + +const POINTER_ID = 1; + +/** + * A primary-button click dispatched the way a browser dispatches it + * (objectui#11546). + * + * `fireEvent.click` alone skips the press, and happy-dom records + * `setPointerCapture` without routing any event by it. A browser sends the + * pointerup and the click to the element that took pointer capture during the + * press, then releases the capture. That routing is what hid objectui#11546 + * from the flow canvas pins: in Chromium, the canvas background captured a + * node press on a read-only canvas, the click landed on the viewport, and the + * node's own select never ran. This helper does that routing, so a press that + * leaks to the background fails the pin the way it fails in a browser. + */ +export function browserClick(target: HTMLElement): void { + const at = { button: 0, pointerId: POINTER_ID, clientX: 10, clientY: 10 }; + fireEvent.pointerDown(target, at); + const captor = capturing() ?? target; + fireEvent.pointerUp(captor, at); + fireEvent.click(captor, { button: 0 }); + releaseAll(); +} + +/** + * A primary-button press that moves before it is released, routed the same + * way. Returns the element that held pointer capture during the move, or null + * when nothing captured the press. + */ +export function browserDrag(target: HTMLElement, dx: number, dy: number): HTMLElement | null { + fireEvent.pointerDown(target, { button: 0, pointerId: POINTER_ID, clientX: 0, clientY: 0 }); + const captor = capturing(); + const routed = captor ?? target; + fireEvent.pointerMove(routed, { pointerId: POINTER_ID, clientX: dx, clientY: dy }); + fireEvent.pointerUp(routed, { button: 0, pointerId: POINTER_ID, clientX: dx, clientY: dy }); + releaseAll(); + return captor; +} + +function capturing(): HTMLElement | null { + return Array.from(document.body.querySelectorAll('*')).find((el) => el.hasPointerCapture?.(POINTER_ID)) ?? null; +} + +/** A browser releases capture implicitly after pointerup; happy-dom does not. */ +function releaseAll(): void { + for (const el of Array.from(document.body.querySelectorAll('*'))) { + if (el.hasPointerCapture?.(POINTER_ID)) el.releasePointerCapture(POINTER_ID); + } +} diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnlySelect-11546.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnlySelect-11546.test.tsx new file mode 100644 index 0000000000..249b913241 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnlySelect-11546.test.tsx @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11546 — on a read-only package, a click on a flow canvas node in the + * Automations pillar selects it and opens the flow inspector read-only. + * + * Measured in Chromium before the fix: the node's pointer-down returned early + * on a read-only canvas, before stopping propagation, so the press reached the + * canvas background. The background cleared the selection and took pointer + * capture on the viewport, so the browser fired the click at the viewport and + * the node's own select never ran. The inspector rail kept its empty state. + * + * objectui#11124's pins never saw it: they send `fireEvent.click` straight to + * the node, with no press first, and happy-dom honours no pointer capture. The + * click below is dispatched the way a browser dispatches it (`browserClick`), + * so the press and the capture are part of what is measured. + * + * The canvas and the inspector are the REAL registered `FlowPreview` and + * `FlowInspector`. The writable control is in the same file. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const PKG = 'com.acme.app'; + +const FLOW = { + name: 'notify_owner', + label: 'Notify owner', + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], +}; + +const server = vi.hoisted(() => ({ + active: new Map>(), + saves: [] as Array<{ type: string; name: string }>, +})); + +const mockClient = vi.hoisted(() => { + const k = (type: string, name: string) => `${type}/${name}`; + return { + list: vi.fn(async (type: string) => + [...server.active.entries()] + .filter(([key]) => key.startsWith(`${type}/`)) + .map(([, row]) => ({ name: row.name, label: row.label ?? row.name })), + ), + listDrafts: vi.fn(async () => []), + listTypes: vi.fn(async () => ({ entries: [] })), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + layered: vi.fn(async (type: string, name: string) => { + const eff = server.active.get(k(type, name)) ?? null; + return { code: null, overlay: eff, overlayScope: eff ? 'env' : null, effective: eff, editable: true, deletable: true, resettable: false, lock: 'none' }; + }), + getDraft: vi.fn(async (type: string, name: string) => { + throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 }); + }), + save: vi.fn(async (type: string, name: string, item: unknown) => { + server.saves.push({ type, name }); + return { type, name, item }; + }), + publish: vi.fn(async () => ({ success: true })), + reset: vi.fn(async () => ({})), + }; +}); + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useMetadataClient: () => mockClient, useMetadataTypes: () => ({ entries: [] }) }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => []) }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => dataSource }; +}); + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { AutomationsPillar } from './StudioDesignSurface'; +import { createEmptyDataSource, failOnAbsorbedFetchError } from './__tests__/emptyDataSource'; +import { registerMetadataPreview } from '../metadata-admin/preview-registry'; +import { registerMetadataInspector } from '../metadata-admin/inspector-registry'; +import { FlowPreview } from '../metadata-admin/previews/FlowPreview'; +import { FlowInspector } from '../metadata-admin/inspectors/FlowInspector'; +import { browserClick } from '../metadata-admin/previews/__tests__/browserClick'; + +const dataSource = createEmptyDataSource(); +failOnAbsorbedFetchError(); + +// The pillar's `/automation/_status` probe and the inspector's action-catalog +// read both go through the global `fetch`; one module-scope double answers +// "absent" so each keeps its documented fallback. +vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })), +); + +registerMetadataPreview('flow', FlowPreview); +registerMetadataInspector('flow', FlowInspector); + +beforeEach(() => { + server.active.clear(); + server.saves.length = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + server.active.set(`flow/${FLOW.name}`, JSON.parse(JSON.stringify(FLOW))); +}); + +afterEach(cleanup); + +function renderPillar(readOnly: boolean) { + return render( + + + , + ); +} + +async function openFlow(): Promise { + await waitFor(() => expect(screen.getByText('Status:').nextElementSibling?.textContent).toBe('active'), { timeout: 8000 }); +} + +function startNodeCard(): HTMLElement { + const card = document.querySelector('[data-node-id="start"] [role="button"]') as HTMLElement | null; + expect(card, 'the canvas must render the start node').not.toBeNull(); + return card!; +} + +/** The controls in the rail an author could type into or toggle, and that are not disabled. */ +function enabledRailControls(rail: HTMLElement): HTMLElement[] { + return Array.from( + rail.querySelectorAll( + 'input, textarea, select, [contenteditable="true"], [role="combobox"], [role="switch"], [role="checkbox"]', + ), + ).filter((el) => !el.matches(':disabled') && el.getAttribute('aria-disabled') !== 'true'); +} + +describe('Automations pillar on a read-only package: a node click opens the inspector read-only (objectui#11546)', () => { + it('selects the clicked node and opens its inspector, with every input disabled', async () => { + renderPillar(true); + await openFlow(); + + browserClick(startNodeCard()); + + const rail = screen.getByRole('complementary'); + const label = await within(rail).findByLabelText('Label', undefined, { timeout: 8000 }); + expect(startNodeCard()).toHaveAttribute('aria-pressed', 'true'); + expect(label).toBeDisabled(); + expect(within(rail).getByLabelText('ID')).toBeDisabled(); + expect(enabledRailControls(rail)).toEqual([]); + expect(server.saves).toHaveLength(0); + }); +}); + +describe('Automations pillar on a writable package — the control (objectui#11546)', () => { + it('the same click opens the inspector with editable inputs', async () => { + renderPillar(false); + await openFlow(); + + browserClick(startNodeCard()); + + const rail = screen.getByRole('complementary'); + const label = await within(rail).findByLabelText('Label', undefined, { timeout: 8000 }); + expect(startNodeCard()).toHaveAttribute('aria-pressed', 'true'); + expect(label).toBeEnabled(); + // The read-only case's "no enabled control" reading can fail: here it finds some. + expect(enabledRailControls(rail).length).toBeGreaterThan(0); + }); +});