Skip to content

fix(draw): restore table cell resize origin - #1160

Merged
pubuzhixing8 merged 7 commits into
developfrom
xingxing/table-resize-origin-interaction-test
Jul 28, 2026
Merged

fix(draw): restore table cell resize origin#1160
pubuzhixing8 merged 7 commits into
developfrom
xingxing/table-resize-origin-interaction-test

Conversation

@pubuzhixing8

Copy link
Copy Markdown
Collaborator

Summary

Replaces the closed draft #1159 with a clean branch based on the latest develop after #1151 landed.

This fixes the table cell resize-origin case: when a cell edge is dragged away from its original size and then the pointer returns to the pointer-down position within the same resize cycle, the table should restore the original row/column size and points.

Root Cause

resizeRef.element is captured on pointer down and remains the pointer-down element reference during the resize cycle. The board update flow is immer-backed: set_node mutates a draft and then replaces board.children, so resizeRef.element still represents the original resize baseline.

When the cumulative pointer offset returns to 0, the computed target size is the original size and appliedOffset becomes 0. The previous appliedOffset !== 0 guard skipped that write, leaving the board at the previous non-zero resize frame.

Change

Remove the appliedOffset !== 0 guard so the resize path applies the computed row/column state even when the effective offset is zero.

Test Coverage

Added an interaction-style spec for withTableResize that uses the real withDraw plugin chain and simulated pointer events:

  1. Select a table.
  2. Pointer down on the first column right edge.
  3. Pointer move from 100 to 120 and verify the first column expands to 120.
  4. Pointer move back to the original pointer-down position and verify the first column and table points restore to the original state.

Verification

  • npm run build:draw

Attempted locally but blocked by existing/local test environment issues:

  • npx tsc -p packages/draw/tsconfig.spec.json --noEmit still fails on existing swimlane spec issues unrelated to this change:
    • packages/draw/src/transforms/swimlane.spec.ts: flatMap lib / implicit any errors
    • packages/draw/src/utils/swimlane.spec.ts: BOARD_TO_TEMPORARY_POINTER export error
  • npm run test:draw -- --watch=false --browsers=ChromeHeadlessCI --progress=false --include=packages/draw/src/plugins/with-table-resize.spec.ts exits locally during the Karma building phase with code 134 before running specs.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 24, 2026

Copy link
Copy Markdown

Deploying plait with  Cloudflare Pages  Cloudflare Pages

Latest commit: bc0690a
Status: ✅  Deploy successful!
Preview URL: https://c25d56c4.plait.pages.dev
Branch Preview URL: https://xingxing-table-resize-origin-t7hs.plait.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 24, 2026

Copy link
Copy Markdown

Deploying plait-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: bc0690a
Status: ✅  Deploy successful!
Preview URL: https://61b08ce7.plait-docs.pages.dev
Branch Preview URL: https://xingxing-table-resize-origin-nfs4.plait-docs.pages.dev

View logs

@pubuzhixing8
pubuzhixing8 marked this pull request as ready for review July 25, 2026 09:00
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Hi @nightt5879 , Please help review, I also improve text fixture in this PR!

@nightt5879 nightt5879 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up. The overall direction looks good, and I left the higher-priority behavior and fixture notes inline. Two small, non-blocking cleanup notes:

  • [P3] fixture.destroy() only traverses the current board.children, so initial nodes replaced by Immer can retain NODE_TO_PARENT / NODE_TO_INDEX entries. Keeping the initial snapshot (or all mapped nodes) for cleanup would improve test isolation.
  • [P3] fill-style.spec.ts calls the same GeometryShapeGenerator.draw() twice at lines 61-62; one of the calls can be removed.

Comment thread packages/draw/src/plugins/with-table-resize.spec.ts
Comment thread packages/draw/src/plugins/with-table-resize.ts Outdated
Comment thread packages/draw/src/plugins/with-table-resize.ts
Comment thread packages/core/src/testing/fixture/mock-host.ts Outdated
Comment thread packages/core/src/testing/fixture/mock-host.ts Outdated
@pubuzhixing8

pubuzhixing8 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Also addressed the two non-blocking cleanup notes from the review in 2e45e4e:

  • fixture.destroy() now keeps the initial children snapshot and clears WeakMap entries for both the initial and current children, with regression coverage.
  • Removed the duplicate GeometryShapeGenerator.draw() call from fill-style.spec.ts.

@nightt5879 Ready for next reviewing.

@nightt5879

Copy link
Copy Markdown
Collaborator

Thanks for the update. I went through the latest commit and the full diff again. The previous viewBox, RoughSVG, WeakMap cleanup, and duplicate draw() issues look fixed.

A few things still need a look:

  • P1: The pending resize frame can still be lost if pointerUp happens before RAF. fix(common): flush pending resize frame on pointer up #1161 may fix this, but it is not part of this PR yet.
  • P2: Existing sizes below 20 cannot return to their original value after resizing.
  • P2: A resize clamped to the current minimum still creates a history entry and clears redo.
  • P2: After the first setNode, setupTestingBoard keeps stale node mappings and selection references.

Two minor notes: destroy() still leaves some fixture caches behind, and the RoughSVG mock is typed as the full interface without implementing all methods.

CI is green, but I’d like to see the P1/P2 cases checked before approval.

@pubuzhixing8

pubuzhixing8 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. The latest commit c5292de addresses the remaining fixture-related feedback:

  • setupTestingBoard now refreshes node WeakMap mappings and selected-element references after operations, and destroy() clears all mapped-node entries plus board-level fixture caches. Regression coverage was added.
  • The RoughSVG fixture now uses a complete RoughSVG instance instead of casting a partial mock.
  • The earlier non-blocking fixture cleanup and duplicate draw() cleanup remain included.

For the items intentionally not handled in this PR:

  • The pending RAF-on-pointer-up lifecycle concern is being discussed in the related issue, so it is not treated as a dependency or blocker for this PR.
  • Existing explicit sizes below 20 are historical data; we are not adding backward-compatibility behavior for those values for now.
  • No-op set_node operations from min-size clamping are now skipped by comparing the computed rows/columns and points against the live table before calling setNode, so a true no-op no longer creates a history entry or clears redo. I avoided keying this off appliedOffset === 0 because that value is ambiguous here: it can mean either the pointer returned to the original position, or the resize was clamped by MIN_CELL_SIZE. Comparing the final computed data is more direct and easier to reason about.
  • Once a row or column has been dragged, it intentionally remains explicitly sized; returning to the origin restores its numeric size rather than the prior auto-size representation.

Core type-check/build and draw build pass locally.

@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

@nightt5879 Please help me review again!

@nightt5879 nightt5879 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the updates!

@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

@nightt5879 Thank you, finally I can merge it.

@pubuzhixing8
pubuzhixing8 merged commit cd9fa2c into develop Jul 28, 2026
1 check passed
@pubuzhixing8
pubuzhixing8 deleted the xingxing/table-resize-origin-interaction-test branch July 28, 2026 13:24
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.

2 participants