Commit 278d244
fix(app-shell): a node click on a read-only flow canvas selects the node and opens the inspector read-only (objectui#11546) (#11603)
Fixes #11546
Clause-②: no
On a read-only package, a click on a flow canvas node in Studio →
Automations now selects the node and opens the flow inspector read-only.
Only the drag is withheld. No editable input appears.
## Mechanism (measured in Chromium before the fix)
`FlowCanvas`'s `onNodePointerDown` returned early on a read-only canvas
(`if (!editable || e.button !== 0) return;`) before
`e.stopPropagation()`. The press therefore reached the viewport's
`onBgPointerDown`, which calls `onSelect(null)` and takes pointer
capture on the viewport. A browser sends the pointerup and the click to
the capturing element, so the click landed on the viewport and the node
card's `onClick` (its select) never ran.
A scratch harness (untracked, deleted after use) mounted the registered
`FlowPreview` exactly as the Automations pillar mounts it on a read-only
package (`editing: true`, `onSelectionChange`, no `onPatch`), with the
registered `FlowInspector` given `readOnly: true`. It was driven in
Chromium (`/opt/pw-browsers/chromium`) at base `2e818d0` and again with
the fix:
| reading | base `2e818d0` | with the fix |
|---|---|---|
| read-only node click (`page.click`) | selection `none`, inspector
absent | `node:find`, 7 inputs, 0 enabled |
| read-only node click (`mouse.down` / `mouse.up`) | selection `none` |
`node:find` |
| event targets after a read-only node press | pointerdown on the node
card, `gotpointercapture` / pointerup / click on the viewport
(`role=application`) | pointerdown, pointerup and click on the node card
|
| read-only press-and-drag on a node | not moved | not moved |
| read-only edge click | `edge:e_start_find`, 3 inputs, 0 enabled | same
|
| editable node click / drag (control) | `node:find`, 7 inputs, 7
enabled / moved | same |
## Change
`FlowCanvas` `onNodePointerDown`: in design mode a node press stops
propagation whether or not the canvas is editable, and only the drag
start stays gated on `editable`. The Delete-key handler is untouched and
still gated on `editable`. Outside design mode, where a node click
selects nothing, a node press still falls through to the background pan,
as before.
`StudioDesignSurface.tsx` is unchanged. The selection already opened the
inspector with the pillar's real `readOnly` flag (objectui#11124's
path). It was the selection that never happened.
**Edges are a different mechanism and needed nothing.** The edge
hit-path and its branch-label pill stop propagation on pointer-down
unconditionally whenever the edge is selectable (design mode with
`onSelectEdge`). The table above shows a read-only edge click selecting
the edge at base already. Nested region-tray nodes also stop propagation
unconditionally.
## Pins
- New
`packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnlySelect-11546.test.tsx`,
with the real `AutomationsPillar`, `FlowPreview` and `FlowInspector`:
- "selects the clicked node and opens its inspector, with every input
disabled": a read-only package; the rail's Label and ID are disabled,
and no input, textarea, select, contenteditable, combobox, switch or
checkbox in the rail is enabled. No save happens.
- "the same click opens the inspector with editable inputs": the
writable control. It also shows that the read-only test's "no enabled
control" reading can fail.
- `FlowCanvas.test.tsx`, a new describe block for objectui#11546: a
read-only click selects that node and never calls `onSelect(null)`; a
read-only drag captures nothing, writes nothing and pans nothing; the
editable designer (the control) still selects on click and commits
`position` on drag; outside design mode a node press still pans.
- New helper
`packages/app-shell/src/views/metadata-admin/previews/__tests__/browserClick.ts`
(`browserClick`, `browserDrag`). It sends the pointerup and the click to
the element holding pointer capture, then releases capture, as a browser
does. happy-dom records `setPointerCapture` but routes no event by it.
That routing gap is why objectui#11124's pins stayed green on this
defect: they send `fireEvent.click` straight to the node card, with no
press. The helper sits under `__tests__/`, which app-shell's
`tsconfig.json` excludes from the build. The dist check below confirms
nothing of it ships.
- objectui#11124's read-only-inputs pin, "opens the flow inspector
read-only: the node inputs are disabled" in
`StudioDesignSurface.automationsReadOnly-11124.test.tsx`, is untouched
and green, together with the rest of that file.
## Ablation (fix committed first; the subject resolves from `src`, so no
build sits on the path)
The ablation used `node
/home/user/objectstack/scripts/ablation-replace.mjs` under the verify
lock. The anchor was the three fixed lines (`if (!editable &&
!designMode) return;` / `e.stopPropagation();` / `if (!editable)
return;`). They were replaced by the base order (`if (!editable)
return;` / `e.stopPropagation();`). The tool reported: anchor hits 1 →
0, blob `82501d693e1d` → `95126d94e35d`.
- Expected direction: the read-only pins turn red and the controls stay
green. **Observed: that direction.** `Tests 3 failed | 29 passed (32)`
over the new pin file, `FlowCanvas.test.tsx` and the objectui#11124
file. The 3 red were the host read-only pin (the inspector's Label never
appears), the canvas read-only click (`expected [ null ] to deeply equal
[ 'b' ]`) and the canvas read-only drag (the viewport captured the
press). Every objectui#11124 pin stayed green under the defect.
- Restore: `git checkout HEAD -- PATH` was run with PATH absolute. Blob
after the restore = HEAD blob
`82501d693e1d58384729c3cf61f9c29284399a98`, and `git diff HEAD` is
empty.
## Gates (all at `c47b708`)
| command | exit | verdict line |
|---|---|---|
| `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...'
build` (lock) | 0 | `Scope: 29 of 47 workspace projects`, `VERDICT
command-exit 0` |
| `pnpm --filter @object-ui/app-shell type-check` (`tsc --noEmit && tsc
-p tsconfig.test.json`) (lock) | 0 | silent tsc; `--listFilesOnly` on
`tsconfig.test.json` lists the helper and both pin files (3) |
| `pnpm exec vitest run --maxWorkers=2
packages/app-shell/src/views/metadata-admin/
packages/app-shell/src/views/studio-design/` +
`packages/app-shell/src/__tests__/spec-symbol-parity.test.ts`,
`apps/console/src/__tests__/registry-inputs-spec-parity.test.ts`,
`scripts/__tests__/{markdown-test-inputs,one-authority-per-exported-name-6273,check-lucide-icon-record-names}.test.ts`
(lock) | 0 | `Test Files 472 passed (472)`, `Tests 5352 passed / 1
skipped (5353)` |
| `pnpm --filter @object-ui/app-shell build`, then a `dist/` probe
(lock) | 0 | `FlowCanvas.js` 1 (control, carries the fix);
`browserClick*` 0; `__tests__` paths 0; `*11546*` 0 |
| `node scripts/check-control-bytes.mjs` | 0 | `OK (scanned 7497 tracked
text file(s)…)` |
| `node scripts/check-new-cross-file-line-citations.mjs` | 0 | `0 new
citation(s)` |
| `node scripts/check-changeset-presence.mjs` | 0 | `1 changeset(s)
added` |
| `node scripts/check-changeset-claims.mjs` | 0 | `No pending changeset
names a file this change touches.` |
| `node scripts/check-pending-changeset-literals.mjs` | 0 | `No test
source names a pending changeset.` |
| `node scripts/check-changeset-no-major.mjs` /
`check-changeset-overwrite.mjs` | 0 / 0 | `No changeset declares a major
bump.` / `No pre-existing changeset was modified or deleted.` |
| `node scripts/check-vi-mock-specifiers.mjs` /
`check-vi-mock-inherit.mjs` / `check-vi-mock-override-shape.mjs` | 0 / 0
/ 0 | `OK` each |
| `node scripts/check-test-path-roots.mjs` | 0 | `OK` |
| `node scripts/check-phantom-dependencies.mjs` /
`check-published-tsconfig-tooling-exclude.mjs` | 0 / 0 | `Every in-scope
i…` / `all 34 enforced package(s) carry the directory form` |
**Test selection, narrowed:** both touched directories in full, plus
every test that imports or reads as source text a changed file or
`FlowCanvas`. The selection was made by `git grep` for `FlowCanvas` /
`flow-canvas-parts` / `browserClick` over test files, and by
`readFileSync` / `?raw` readers that name the previews path. The rest of
app-shell and the whole-repo `pnpm test` are declared to CI.
**Lint, a proven narrowing (not `pnpm lint`):** `npx eslint --format
json` was run from `packages/app-shell`, as its `eslint .` would be.
1. Population: the diff's four `.ts`/`.tsx` files. eslint's own config
answers the changeset `.md` with "File ignored because no matching
configuration was supplied."
2. Count: the JSON holds 4 results, 0 errors and 1 warning. The warning
is `react-hooks/exhaustive-deps` on `addNode`'s `useCallback`. The base
file linted identically shows the same warning at the same place, so it
is not this diff's.
3. Invariance: `eslint.config.js` sets no `parserOptions.project` /
`projectService`, so the linting is not type-aware. No rule under
`eslint-rules/` reads another file (zero `readFileSync` / `readdirSync`
/ `globSync` hits). This diff therefore cannot move the verdict on an
untouched file.
NOT MEASURED: `pnpm check:published-dist`, reason: it builds the whole
workspace (CI's run). The narrowed `dist/` probe above covers this
diff's helper. NOT MEASURED: full `pnpm test` / `pnpm lint`, reason:
repository-wide runs owned by CI.
## Acceptance notes
- The `react-hooks/exhaustive-deps` warning on `addNode` (an unnecessary
`positionOf` dependency) predates this branch and is left alone. It is
out of scope and not a defect.
- One-off evidence (the Chromium harness, the driver, the ablation) left
no file in the tree.
Session: `https://claude.ai/code/session_01FjqrwXPfSMkSfkKYDSRkN2`
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FjqrwXPfSMkSfkKYDSRkN2)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent b61c116 commit 278d244
5 files changed
Lines changed: 349 additions & 2 deletions
File tree
- .changeset
- packages/app-shell/src/views
- metadata-admin/previews
- __tests__
- studio-design
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
Lines changed: 87 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
499 | 500 | | |
500 | 501 | | |
501 | 502 | | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
Lines changed: 11 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
407 | 407 | | |
408 | 408 | | |
409 | 409 | | |
410 | | - | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
411 | 419 | | |
| 420 | + | |
412 | 421 | | |
413 | 422 | | |
414 | 423 | | |
| |||
420 | 429 | | |
421 | 430 | | |
422 | 431 | | |
423 | | - | |
| 432 | + | |
424 | 433 | | |
425 | 434 | | |
426 | 435 | | |
| |||
Lines changed: 53 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
Lines changed: 181 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
0 commit comments