Skip to content

feat(core): adjust table resize#931

Open
huanhuanwa wants to merge 5 commits into
developfrom
adjust-table-resize
Open

feat(core): adjust table resize#931
huanhuanwa wants to merge 5 commits into
developfrom
adjust-table-resize

Conversation

@huanhuanwa

Copy link
Copy Markdown
Contributor

No description provided.

@vercel

vercel Bot commented Jul 1, 2024

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plait ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 2, 2024 3:13am
plait-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 2, 2024 3:13am

Comment thread packages/draw/src/plugins/with-table-resize.ts Outdated
});
}

function getResizeCellDirection(handleIndex: number) {

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.

start 和 end 概念的抽取需要讨论下

return index;
}

function getResizeColumnOrRowSize(board: PlaitBoard, element: PlaitBaseTable, index: number, isRow: boolean) {

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.

getCurrentRowHeightOrColumnWidth vs getMinColumnWidthOrRowHeight

@nightt5879

Copy link
Copy Markdown
Collaborator

This PR has been inactive for a long time. Should we keep it open, or close it if it is no longer planned?

@pubuzhixing8

Copy link
Copy Markdown
Collaborator

we could keep it open, I plan to handle it lately.

@pubuzhixing8

Copy link
Copy Markdown
Collaborator

I took another pass over this PR to understand the original intent and whether it is still mergeable.

What this PR is trying to fix:

  • It changes table resize hit testing so cell edge handles are preferred over the whole-table side handles. This makes resizing the outer row/column edge behave like a row/column resize instead of scaling the whole table.
  • It introduces a start / end direction so dragging the left/top edge updates table.points[0], while dragging the right/bottom edge updates table.points[1].
  • It tries to handle merged cells by resizing the first or last row/column covered by a rowspan / colspan.
  • It keeps whole-table proportional resizing only for corner handles.

I think the intent is useful, but I would not merge this PR as-is:

  • The branch is very stale now. It is 321 commits behind develop and GitHub reports it as not mergeable.
  • The current develop table resize code has changed since this PR was opened, especially around canResize, selected-table-only hit testing, rotated table guards, and text/table utilities. A direct merge would likely regress some of that newer behavior.
  • The span handling is still a bit fragile. getResizeColumnOrRowSize infers the current row/column size from cell rectangles, which can be wrong or return 0 when the target row/column is only represented through merged cells. This matches the earlier review concern around whether this should be current size vs min size.
  • The start / end concepts are valid, but they should probably be modeled more explicitly around resize handle direction instead of being local string flags.
  • There are no tests covering left/top resize, right/bottom resize, and rowspan / colspan cases, which are exactly the risky parts of this change.

My recommendation: close this PR and port the useful behavior in a fresh PR against current develop. The fresh implementation should preserve the current selection / rotation guards, keep the cell-first hit testing behavior if that is still desired, compute row/column sizes from the table layout model instead of from arbitrary cell rectangles, and add focused tests for normal cells plus merged-cell resize from both start and end edges.

@pubuzhixing8 pubuzhixing8 reopened this Jul 4, 2026
@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying plait-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9bd1863
Status: ✅  Deploy successful!
Preview URL: https://3609af7a.plait-docs.pages.dev
Branch Preview URL: https://adjust-table-resize.plait-docs.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying plait with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9bd1863
Status: ✅  Deploy successful!
Preview URL: https://18a99d30.plait.pages.dev
Branch Preview URL: https://adjust-table-resize.plait.pages.dev

View logs

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.

3 participants