Skip to content

fix(draw): adjust table cell edge resize - #1151

Merged
pubuzhixing8 merged 2 commits into
developfrom
codex/table-resize-cell-edges
Jul 24, 2026
Merged

fix(draw): adjust table cell edge resize#1151
pubuzhixing8 merged 2 commits into
developfrom
codex/table-resize-cell-edges

Conversation

@pubuzhixing8

@pubuzhixing8 pubuzhixing8 commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR reworks the table resize interaction from the older #931 idea against the current develop branch.

The important behavior change is not just a code cleanup. It makes table resize intent explicit:

cell side edge -> resize the corresponding row/column
table corner -> resize the whole table
cell start edge -> resize the row/column from the start side
cell end edge -> resize the row/column from the end side
merged-cell start edge -> resize the first covered row/column
merged-cell end edge -> resize the last covered row/column

Interaction Intent Changes

1. Outer table edges now mean row/column resize, not whole-table resize

Before this change, when a user selected a table and moved the pointer to the right edge of the table, that position could match both:

  • the table right resize handle
  • the last cell right resize handle

The old hit-test path checked the table handle first, so the interaction was often interpreted as whole-table resize.

That meant the user appeared to be dragging the last column edge, but the system treated it as resizing the whole table.

After this change, cell resize handles are checked first. If the pointer hits the right edge of the last cell, the interaction is handled as a column resize.

Before: drag table right edge -> resize the whole table
After:  drag table right edge -> resize the last column

2. Whole-table resize is now reserved for table corners

Before this change, both table side handles and table corner handles could trigger whole-table resize.

That made these two user intents compete with each other:

  • resizing a row/column
  • resizing the whole table

After this change, table-level resize only falls back to corner handles. Side edges are reserved for row/column resize.

Side edge -> row/column resize
Corner    -> whole-table resize

This makes the interaction model mutually exclusive and easier to predict.

3. Start-side row/column resize is now explicit

The previous implementation could already move points[0] through the table-level resize path. For example, when the interaction was handled as a whole-table resize, resizeSnapRef.activePoints plus normalizeShapePoints could produce a new start point.

The gap was in the cell row/column resize path. The old row/column update helpers behaved like end-side updates:

column width changes -> update points[1][0]
row height changes   -> update points[1][1]

That is correct for right/bottom cell edges, but it does not clearly model the user intent when dragging a left/top cell edge.

When a user drags the left edge of the first column, the expected intent is not whole-table resize. The intent is:

move the table left boundary and resize the first column

So the row/column resize path itself needs to know whether the dragged edge is the start side or the end side.

This PR introduces a small ResizeEdge concept:

  • w / n -> start
  • e / s -> end

So the row/column update now matches the dragged cell edge:

left cell edge   -> update points[0][0] while resizing the target column
right cell edge  -> update points[1][0] while resizing the target column
top cell edge    -> update points[0][1] while resizing the target row
bottom cell edge -> update points[1][1] while resizing the target row

The improvement is therefore not first-time support for changing points[0]; it is making start-side row/column resize explicit instead of relying on the table-level resize path.

4. Merged cells resolve the resize target by dragged edge

Merged cells can cover multiple rows or columns.

For example, a cell with colspan=3 covers columns 1, 2, and 3.

The expected interaction is:

drag the left edge  -> resize column 1
drag the right edge -> resize column 3

The old logic mainly followed the cell starting rowId / columnId, which made end-side resize ambiguous for rowspan / colspan cells.

This PR resolves the target row/column from the dragged edge:

start edge -> first row/column covered by the cell
end edge   -> last row/column covered by the cell span

So merged-cell resize now maps to the user visible drag target instead of always leaning on the cell starting row/column.

Implementation Notes

  • hitTest checks cell side handles before table-level handles.
  • table-level fallback only accepts corner handles.
  • ResizeEdge represents whether the dragged cell edge is the start or end side.
  • getResizeTargetRowOrColumnIndex resolves the row/column affected by normal and merged cells.
  • getCurrentRowOrColumnSize derives the current rendered row/column size from the table layout, avoiding fragile inference from cell rectangles.

Verification

  • npm run build:draw

Focused manual checks to run:

  • Resize the right/bottom edge of a normal table cell and verify only the target column/row is updated.
  • Resize the left/top cell edge and verify the target row/column resizes from the start side.
  • Resize a table corner and verify whole-table resize still works.
  • Resize the start/end edge of a merged cell and verify the first/last covered row or column is updated.

Unit tests can be added in a follow-up once the interaction behavior is confirmed.

@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying plait with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1dd0ec5
Status: ✅  Deploy successful!
Preview URL: https://a06486e8.plait.pages.dev
Branch Preview URL: https://codex-table-resize-cell-edge.plait.pages.dev

View logs

@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying plait-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1dd0ec5
Status: ✅  Deploy successful!
Preview URL: https://3d280576.plait-docs.pages.dev
Branch Preview URL: https://codex-table-resize-cell-edge.plait-docs.pages.dev

View logs

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch from 281b6a4 to 6871efa Compare July 12, 2026 16:18
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

@nightt5879 I think this PR is ready for review, help me, please!

@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.

A few edge cases I ran into while testing this.

Comment thread packages/draw/src/plugins/with-table-resize.ts Outdated
Comment thread packages/draw/src/plugins/with-table-resize.ts Outdated
Comment thread packages/draw/src/plugins/with-table-resize.ts Outdated
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

I am still working on it, because I need to check the new resolution.

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch from d682b87 to 2ad1b16 Compare July 16, 2026 13:00
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Follow-up: I revisited the implementation and narrowed it back to the original cell-edge based approach. The current fix keeps TableResizeOptions cell-based, adds side-only handle detection so corner hits can still be treated as row/column edge resize, chooses among simultaneous cell hits by distance and target row/column index instead of raw cells order, and uses a signed pointer delta with MIN_CELL_SIZE clamping. This avoids the larger divider abstraction while still addressing the review feedback. Thanks again for the good eye here.

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch 2 times, most recently from c7d6edd to 4b9b4a1 Compare July 20, 2026 07:17
@pubuzhixing8

pubuzhixing8 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @nightt5879
Update for review: I kept this PR focused on the table cell edge resize behavior and removed the extra cell-order/stable-hit handling from this change.

The current implementation intentionally stays simple:

  • preserve table corner resize priority;
  • use edge-only handle hit testing for cell resize so cell corner/intersection hits do not fall through to swimlane/table movement;
  • use a signed pointer delta with MIN_CELL_SIZE clamping to avoid reverse growth after crossing the fixed edge.

The persisted cells ordering issue is valid, but it is a broader table/swimlane data invariant rather than resize hit-test logic. I will address it in a separate PR by normalizing cells before saving, keeping the order top-to-bottom and left-to-right. That keeps this PR small and avoids introducing a more complex detection process here.

Help me review again, please

@nightt5879

Copy link
Copy Markdown
Collaborator

@pubuzhixing8

Thanks for the update. I reviewed the current head again. I’m aligned with keeping this PR focused, and I have one merge-sequencing suggestion, one non-blocking finding, and one interaction question.

  1. cells ordering and merge sequencing

    I understand that the persisted cells ordering invariant is intentionally being deferred to a separate PR. I confirmed that this PR on its own can still select the opposite side of a shared divider after inserting a row or column in the middle, because the live cells order can differ from the visual row/column order.

    Since this is a foundational library with downstream consumers, my personal preference would be to merge the normalization PR first, then rebase this PR onto it. That would avoid temporarily exposing resize behavior that depends on cell storage order.

    That said, I’m also okay with the proposed split if the normalization PR follows immediately after this one and the dependency is clearly tracked or linked. Could you clarify the intended merge order?

  2. Non-blocking, pre-existing resize issue

    While testing, I noticed an existing issue in the cell resize path: if a column starts at 100, is dragged to 120, and the pointer is then moved back exactly to the pointer-down position, the column remains at 120.

    The zero cumulative offset causes the update to be skipped, while the board still contains the result from the previous frame. I confirmed that the previous implementation has the same behavior, so I do not consider this a regression introduced by this PR or a blocker here. It would still be worth tracking and covering in a follow-up test.

  3. Grid-intersection interaction question

    Around a grid intersection, the horizontal and vertical edge hit areas can overlap. The current edge-only hit test checks handles in a fixed order, so a point directly on the horizontal divider but also close to the vertical divider can select column resize.

    Is that fixed priority intentional, or should the closest divider axis win when both edge hit areas match? I’m treating this as an interaction question rather than a correctness issue until the intended behavior is confirmed.

Overall, I do not consider points 2 or 3 blockers. For point 1, my main concern is making the merge dependency and follow-up timing explicit rather than expanding the scope of this PR unexpectedly.

@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Thanks for reviewing again.

For point 1, the follow-up PR is ready here: #1157. It normalizes the persisted cells order separately, so this PR can keep the resize logic simple without adding a more complex hit-detection flow.

For point 2, I agree this is a pre-existing resize issue. I will address it in a separate follow-up PR with focused coverage.

For point 3, I agree the grid-intersection priority was not considered at that level of detail originally. The current interaction is acceptable for now, so I would prefer not to add more complex axis-priority handling in this PR.

@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Thanks for the good eye on this resize-origin case.

I fixed it by making the cell-resize path apply the computed target state on every resize frame, including when the cumulative pointer offset returns to zero. This restores the original column width and table points instead of leaving the last non-zero resize result in place.

I also added a focused unit test covering: expand a column from 100 to 120, then move the pointer back to the pointer-down position, and verify the width/points return to the original state.

Validation:

  • npm run build:draw
  • npx tsc -p packages/draw/tsconfig.spec.json --noEmit

Could you please review again when you have a chance?

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch from 2275724 to e40bb1b Compare July 23, 2026 02:12
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Good catch on the test shape. The previous unit test was not close enough to the real interaction because it called the helper twice against the same unchanged table, while the actual resize flow mutates resizeRef.element through set_node during pointer move.

I updated the fix and the test accordingly:

  • beforeResize now captures a resize-origin snapshot for cell resizing.
  • onResize calculates each frame from that origin snapshot instead of the already-mutated active table.
  • The unit test now applies the first resize result back to the active table before checking the zero-offset frame, so it verifies the real mutable-baseline case.

Validation:

  • npm run build:draw
  • npx tsc -p packages/draw/tsconfig.spec.json --noEmit

test:draw still exits locally during the Karma building phase with code 134 before running specs, so I could not use it as a reliable local signal.

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch from e40bb1b to 1d5c59e Compare July 23, 2026 02:22
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Correction: I reverted the unit-test-driven change from the previous update.

After re-checking it, adding a resize-origin snapshot in production code just to make that unit test meaningful was not appropriate for this PR. The branch now keeps the fix minimal and does not add that unit test or the extra helper/snapshot logic.

Current scope:

  • Keep the existing resize flow and detection logic unchanged.
  • Only remove the appliedOffset !== 0 guard so the resize path can still write the computed state when the effective offset is zero.

Validation:

  • npm run build:draw
  • npx tsc -p packages/draw/tsconfig.spec.json --noEmit

@nightt5879

Copy link
Copy Markdown
Collaborator

@pubuzhixing8

I rechecked the latest commit. Removing the appliedOffset !== 0 guard alone does not restore the resize origin: set_node mutates resizeRef.element, so after 100 -> 120, returning the cumulative pointer offset to 0 reads currentSize as 120 and writes 120 again.

Since this was non-blocking, I’m fine with reverting the latest commit and handling it separately; otherwise, the fix needs an immutable pointer-down baseline (or equivalent) and a two-frame test.

Also, although the PR is currently conflict-free, the head is still based before the latest develop (including #1157). Please rebase onto the current develop before the final review.

@pubuzhixing8
pubuzhixing8 force-pushed the codex/table-resize-cell-edges branch 2 times, most recently from 1dd0ec5 to 51a2a3e Compare July 23, 2026 07:29
@pubuzhixing8

Copy link
Copy Markdown
Collaborator Author

Final update for this review round.

I rechecked the resize-origin issue with the board update model in mind. The correct reasoning is:

  • 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.
  • Therefore, when the cumulative pointer offset returns to 0, the computed target size is the original size and appliedOffset becomes 0.
  • The appliedOffset !== 0 guard then skips the write, leaving the board at the previous non-zero resize frame.

So the issue is real, but it is pre-existing and not part of the main interaction scope of this PR. I have left this PR unchanged and moved that focused fix into a separate follow-up PR:

#1159

That follow-up is based on this PR branch so its diff only contains the resize-origin fix. After this PR lands, it can be retargeted or rebased onto develop if needed.

This PR is now still focused on the table cell edge resize interaction, and it is already based on the current develop that includes #1157.

@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 clarifying the immer-backed update model. I agree that the resize-origin issue is pre-existing and better handled separately in #1159, while keeping this PR focused.

@pubuzhixing8
pubuzhixing8 merged commit e57a152 into develop Jul 24, 2026
3 checks passed
@pubuzhixing8
pubuzhixing8 deleted the codex/table-resize-cell-edges branch July 24, 2026 03:41
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