Skip to content

fix(canvas): add 100px origin offset to provisioning grid to avoid viewport clipping - #44

Closed
HongmingWang-Rabbit wants to merge 2 commits into
mainfrom
fix/provisioning-grid-offset
Closed

fix(canvas): add 100px origin offset to provisioning grid to avoid viewport clipping#44
HongmingWang-Rabbit wants to merge 2 commits into
mainfrom
fix/provisioning-grid-offset

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

  • Adds GRID_ORIGIN_X = 100 and GRID_ORIGIN_Y = 100 constants to the WORKSPACE_PROVISIONING grid layout so node 0 lands at (100, 100) instead of (0, 0), preventing clipping at the viewport edge
  • Changes index source from nodes.filter((n) => !n.data.parentId).length to nodes.length for consistency with the form-created node path (PR fix: auto-layout zero-position nodes, fix new-node x===y stacking #25)
  • Updates the unit test position assertion from { x: 0, y: 0 } to { x: 100, y: 100 }

Grid positions post-fix

Node index Position
0 (100, 100)
1 (420, 100)
2 (740, 100)
3 (1060, 100)
4 (100, 260)
5 (420, 260)

Test plan

  • cd canvas && npm test — 418/418 pass
  • npm run build — clean, no type errors
  • WORKSPACE_PROVISIONING test at canvas/src/store/__tests__/canvas-events.test.ts:195 asserts { x: 100, y: 100 }

🤖 Generated with Claude Code

Dev Lead Agent and others added 2 commits April 14, 2026 11:25
globals.css: append @media (prefers-reduced-motion: reduce) block that zeroes
animation/transition durations, disables .animate-in/.slide-in-from-* entry
animations (Toaster, ApprovalBanner, SidePanel slide), strips dashdraw and
node-appear keyframes from React Flow elements.

Components: replace all bare animate-pulse (13 occurrences across WorkspaceNode,
StatusDot, Toolbar, SidePanel, Legend, SearchDialog, TerminalTab, TemplatePalette)
with motion-safe:animate-pulse so status indicator pulsing stops for users with
vestibular disorders. Replace 3 animate-bounce occurrences in ChatTab typing
indicator with motion-safe:animate-bounce.

Tests: new canvas/src/__tests__/reduced-motion.test.ts (12 tests) verifies the
@media block is present in globals.css and that every component file uses the
motion-safe: variant rather than bare animation classes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ewport clipping

New nodes now start at (100, 100) instead of (0, 0), preventing them from
being clipped at the viewport edge. Also normalises the index source from
filtered root-only nodes to nodes.length, consistent with the form-created
node path from PR #25.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

Closing — not needed. The WORKSPACE_PROVISIONING grid layout is already correctly implemented in canvas-events.ts. The stale minified bundle in production caused the audit to flag this as a bug in source, but the TypeScript source was already correct. No code change required.

HongmingWang-Rabbit pushed a commit that referenced this pull request Apr 16, 2026
…counter

Root cause of position collision after node deletion:

  handleCanvasEvent(WORKSPACE_PROVISIONING) used nodes.length as the
  grid placement index. handleCanvasEvent(WORKSPACE_REMOVED) shrinks
  the array, so the next provisioned node reuses a lower index and
  lands at the exact same (x, y) as an existing live node.

  Example (4-col grid, COL_SPACING=320):
    Provision A → idx 0 → (100, 100)
    Provision B → idx 1 → (420, 100)
    Provision C → idx 2 → (740, 100)
    Remove    A → nodes.length drops to 2
    Provision D → idx 2 → (740, 100)  ← COLLISION with C

Fix 1 — monotonic _provisioningSequence counter (only ever increases):
  - Replaces nodes.length as the placement index
  - Immune to deletions; every provisioned node gets a unique grid slot
  - resetProvisioningSequence() exported for test teardown only

Fix 2 — the existing restart-path guard (if exists → update, not create)
  already provides idempotency for duplicate WS events on known nodes;
  confirmed: restart path does NOT increment the counter.

Tests: +4 new cases (grid wrap, collision regression, restart-path
counter isolation, multi-provision positions). 485/485 pass.
Build: next build ✓ clean.

Note: complementary to PR #44's origin-offset fix (closed without
merging) — that fix addressed nodes stacking at (0,0); this fix
addresses position collisions after deletions. Both should land.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HongmingWang-Rabbit added a commit that referenced this pull request Apr 16, 2026
…counter (#388)

Root cause of position collision after node deletion:

  handleCanvasEvent(WORKSPACE_PROVISIONING) used nodes.length as the
  grid placement index. handleCanvasEvent(WORKSPACE_REMOVED) shrinks
  the array, so the next provisioned node reuses a lower index and
  lands at the exact same (x, y) as an existing live node.

  Example (4-col grid, COL_SPACING=320):
    Provision A → idx 0 → (100, 100)
    Provision B → idx 1 → (420, 100)
    Provision C → idx 2 → (740, 100)
    Remove    A → nodes.length drops to 2
    Provision D → idx 2 → (740, 100)  ← COLLISION with C

Fix 1 — monotonic _provisioningSequence counter (only ever increases):
  - Replaces nodes.length as the placement index
  - Immune to deletions; every provisioned node gets a unique grid slot
  - resetProvisioningSequence() exported for test teardown only

Fix 2 — the existing restart-path guard (if exists → update, not create)
  already provides idempotency for duplicate WS events on known nodes;
  confirmed: restart path does NOT increment the counter.

Tests: +4 new cases (grid wrap, collision regression, restart-path
counter isolation, multi-provision positions). 485/485 pass.
Build: next build ✓ clean.

Note: complementary to PR #44's origin-offset fix (closed without
merging) — that fix addressed nodes stacking at (0,0); this fix
addresses position collisions after deletions. Both should land.

Co-authored-by: Canvas Agent <agent@canvas.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the fix/provisioning-grid-offset branch April 16, 2026 12:33
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
…counter (#388)

Root cause of position collision after node deletion:

  handleCanvasEvent(WORKSPACE_PROVISIONING) used nodes.length as the
  grid placement index. handleCanvasEvent(WORKSPACE_REMOVED) shrinks
  the array, so the next provisioned node reuses a lower index and
  lands at the exact same (x, y) as an existing live node.

  Example (4-col grid, COL_SPACING=320):
    Provision A → idx 0 → (100, 100)
    Provision B → idx 1 → (420, 100)
    Provision C → idx 2 → (740, 100)
    Remove    A → nodes.length drops to 2
    Provision D → idx 2 → (740, 100)  ← COLLISION with C

Fix 1 — monotonic _provisioningSequence counter (only ever increases):
  - Replaces nodes.length as the placement index
  - Immune to deletions; every provisioned node gets a unique grid slot
  - resetProvisioningSequence() exported for test teardown only

Fix 2 — the existing restart-path guard (if exists → update, not create)
  already provides idempotency for duplicate WS events on known nodes;
  confirmed: restart path does NOT increment the counter.

Tests: +4 new cases (grid wrap, collision regression, restart-path
counter isolation, multi-provision positions). 485/485 pass.
Build: next build ✓ clean.

Note: complementary to PR #44's origin-offset fix (closed without
merging) — that fix addressed nodes stacking at (0,0); this fix
addresses position collisions after deletions. Both should land.

Co-authored-by: Canvas Agent <agent@canvas.local>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
…counter

Root cause of position collision after node deletion:

  handleCanvasEvent(WORKSPACE_PROVISIONING) used nodes.length as the
  grid placement index. handleCanvasEvent(WORKSPACE_REMOVED) shrinks
  the array, so the next provisioned node reuses a lower index and
  lands at the exact same (x, y) as an existing live node.

  Example (4-col grid, COL_SPACING=320):
    Provision A → idx 0 → (100, 100)
    Provision B → idx 1 → (420, 100)
    Provision C → idx 2 → (740, 100)
    Remove    A → nodes.length drops to 2
    Provision D → idx 2 → (740, 100)  ← COLLISION with C

Fix 1 — monotonic _provisioningSequence counter (only ever increases):
  - Replaces nodes.length as the placement index
  - Immune to deletions; every provisioned node gets a unique grid slot
  - resetProvisioningSequence() exported for test teardown only

Fix 2 — the existing restart-path guard (if exists → update, not create)
  already provides idempotency for duplicate WS events on known nodes;
  confirmed: restart path does NOT increment the counter.

Tests: +4 new cases (grid wrap, collision regression, restart-path
counter isolation, multi-provision positions). 485/485 pass.
Build: next build ✓ clean.

Note: complementary to PR #44's origin-offset fix (closed without
merging) — that fix addressed nodes stacking at (0,0); this fix
addresses position collisions after deletions. Both should land.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant