Add table column and row resize via border drag handles#100
Merged
Conversation
Optional field for user-specified minimum row heights in pixels. Undefined means content-based auto height (existing behavior).
resizeColumn() adjusts only the two adjacent columns sharing a border, keeping the total ratio sum at 1.0. setRowHeight() sets a user-specified minimum height in pixels via a new rowHeights array on TableData.
Splice the rowHeights array in insertRow() and deleteRow() to keep it aligned with the rows array. New rows get undefined (auto height). Persist the updated array via updateTableAttrs so the store stays in sync.
User heights act as a floor; content height wins when it exceeds the user value. Rows without a user height use content-based auto sizing.
detectTableBorder() checks if mouse coordinates are within 4px of a resizable border. Column borders take priority at intersections. createDragState() computes min/max pixel bounds for drag clamping.
Reuse the ruler's dragGuideline variable and renderPaintOnly() for table border drag feedback. No new rendering code needed.
Detect border proximity on mousemove (cursor change), initiate drag on mousedown, update guideline position during drag, and apply resize on mouseup. Column resize adjusts adjacent ratios; row resize sets minimum height.
Tree class is exported from @yorkie-js/sdk, not @yorkie-js/react. The wrong import caused "Tree is not a constructor" at runtime when creating a new document.
Add the same getRootTreeNode guard used by other store methods to prevent crash when content is not yet a Tree CRDT instance.
The canvas has cursor:text set directly, which overrides the container's cursor style. Change border hover cursor on the canvas element itself.
📝 WalkthroughWalkthroughAdds Google-Docs-style table resizing: model support for per-row minimum heights, Doc APIs to resize columns and set row heights, border hit-detection and drag-state logic, editor mouse integration with a dashed guideline, layout enforcement of row minimums, and persistence via store updates. Changes
Sequence DiagramsequenceDiagram
actor User as User (Mouse)
participant Editor as TextEditor
participant Detector as Table Resize Detector
participant Layout as Table Layout
participant Doc as Doc API
participant Store as Store
participant Renderer as Renderer
User->>Editor: mousedown on table border
Editor->>Detector: detectTableBorder(localX, localY)
Detector->>Layout: query column/row offsets
Detector-->>Editor: BorderHit / null
alt Border detected
Editor->>Editor: createDragState(hit, layout, mouse)
Note over Editor: store startPixel, min/max bounds
end
User->>Editor: mousemove
alt borderDragState active
Editor->>Editor: clamp currentPixel
Editor->>Editor: onDragGuideline({x|y})
Note over Renderer: paint-only dashed guideline render
else
Editor->>Detector: detectTableBorder for cursor
Editor->>Editor: update canvas cursor style
end
User->>Editor: mouseup
alt borderDragState active
Editor->>Doc: resizeColumn(...) or setRowHeight(...)
Doc->>Store: updateTableAttrs({ cols, rowHeights? })
Store-->>Doc: persisted
Editor->>Renderer: trigger full re-render / layout refresh
end
Editor->>Editor: clear borderDragState & guideline
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Document 6 lessons from the implementation: Yorkie SDK/React version mismatch, bundled class identity for instanceof, multi-canvas container selection, CSS cursor precedence, store refresh semantics, and reusing existing dragGuideline infrastructure.
Contributor
Verification: verify:selfResult: ✅ PASS in 107.9s
Verification: verify:integrationResult: ✅ PASS |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/docs-table-resize.md`:
- Around line 157-176: The fenced code blocks showing the column width and row
height formulas are missing a language tag; update the two fences surrounding
the column-resize snippet (containing deltaRatio, newLeftRatio, newRightRatio,
minRatio, resizeColumn) and the row-height snippet (containing deltaPx,
layoutHeight, newHeight, setRowHeight) to include a language identifier such as
text or typescript (e.g., replace ``` with ```text) so markdownlint MD040 is
satisfied and optional highlighting applied.
In `@packages/docs/src/model/document.ts`:
- Around line 564-566: td.rowHeights is treated as number[] but the code
intentionally stores "unset" entries (undefined/null); remove the unsafe cast
"undefined as unknown as number" by widening the rowHeights element type across
the model and store contract (e.g., change the type used in the Document model,
any TableData/type aliases, and Store.updateTableAttrs signature to accept
number | undefined | null as appropriate) or add a normalization step that
converts persisted nulls to undefined before calling updateTableAttrs; update
all call sites (including td.rowHeights splice at the shown blockId path and the
other occurrence around lines 792-799) so the compiler recognizes and enforces
nullish checks instead of using casts.
In `@packages/docs/src/view/table-resize.ts`:
- Around line 59-71: When initializing a column drag (hit.type === 'column')
ensure we handle the case where leftColStart and rightColEnd produce an invalid
range (minPixel > maxPixel) due to adjacent columns totalling less than 2 *
MIN_COLUMN_WIDTH: detect this after computing leftColStart, rightColEnd,
minPixel and maxPixel and either abort the drag or collapse the bounds by
setting both minPixel and maxPixel to mousePixel (or otherwise early-return a
no-op drag) so the mouse-move clamp won’t force the first commit to violate
MIN_COLUMN_WIDTH; update the logic in the block that returns the column drag
object (references: MIN_COLUMN_WIDTH, leftColStart, rightColEnd, minPixel,
maxPixel, mousePixel, hit.index) accordingly.
In `@packages/docs/src/view/text-editor.ts`:
- Around line 735-751: When starting a table border drag in the mouse handler
(where resolveTableFromMouse, detectTableBorder and createDragState are used and
borderDragState/isMouseDown are set), ensure the editor captures the pointer for
the drag lifetime or installs temporary document/window mousemove/mouseup
listeners so the drag is reliably cleared if the pointer leaves the editor;
specifically, call element.setPointerCapture(event.pointerId) (or attach
document.addEventListener('mousemove'/'mouseup') and clean them up) right after
creating borderDragState and set up a removal path that clears borderDragState
and releases pointer capture (or removes the temporary listeners) on
pointerup/mouseup, and apply the same change to the other drag handling block
referenced around lines 896-943 so stale borderDragState cannot persist when the
pointer is released outside the editor.
- Around line 667-711: resolveTableFromMouse uses a tableOriginY anchored to the
table's first rendered line so localY includes inter-page gaps and can miss
row-border hits on later pages; change the Y rebasing to use the page-local
segment that actually contains the mouse similar to resolveOffsetInCellAtXY.
Locate resolveTableFromMouse, find where tablePageY/tableOriginY and localY are
computed, then determine which paginatedLayout.pages segment contains the mouse
Y (using the same row-aware math as in resolveOffsetInCellAtXY) and subtract
that page-segment origin when computing localY so comparisons against
tl.rowYOffsets and tl.totalHeight use gap-free table coordinates for
hit-testing.
In `@packages/frontend/src/app/docs/yorkie-doc-store.ts`:
- Around line 564-575: updateTableAttrs is updating block.tableData.rowHeights
but then rewrites the block using buildBlockNode which only
serializes/deserializes column widths (cols), so rowHeights is lost on Yorkie
round-trip; modify the table serialization/parsing used by buildBlockNode (and
any helper functions that convert Block/tableData to/from Yorkie nodes) to
include rowHeights in the serialized node shape and ensure parse logic restores
tableData.rowHeights so the value persisted by updateTableAttrs survives
rehydrate and remote sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f439f5cf-5b0f-4d4b-9654-76de193e7ae7
📒 Files selected for processing (15)
docs/design/README.mddocs/design/docs-table-resize.mdpackages/docs/src/model/document.tspackages/docs/src/model/types.tspackages/docs/src/store/memory.tspackages/docs/src/store/store.tspackages/docs/src/view/editor.tspackages/docs/src/view/table-layout.tspackages/docs/src/view/table-resize.tspackages/docs/src/view/text-editor.tspackages/docs/test/model/table.test.tspackages/docs/test/model/types.test.tspackages/docs/test/view/table-layout.test.tspackages/docs/test/view/table-resize.test.tspackages/frontend/src/app/docs/yorkie-doc-store.ts
Comment on lines
+564
to
+566
| if (td.rowHeights) { | ||
| td.rowHeights.splice(atIndex, 0, undefined as unknown as number); | ||
| this.store.updateTableAttrs(blockId, { cols: td.columnWidths, rowHeights: td.rowHeights }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Give rowHeights an element type that matches its runtime contract.
Unset row heights are a first-class state here, but this path still has to smuggle them through undefined as unknown as number. Please widen the element type across the model/store contract (or normalize persisted nulls back to undefined) instead of treating rowHeights as a plain number[], so the compiler helps with the nullish checks that the layout code already depends on.
As per coding guidelines, "Use type annotations in TypeScript to improve code clarity and catch errors early."
Also applies to: 792-799
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/src/model/document.ts` around lines 564 - 566, td.rowHeights is
treated as number[] but the code intentionally stores "unset" entries
(undefined/null); remove the unsafe cast "undefined as unknown as number" by
widening the rowHeights element type across the model and store contract (e.g.,
change the type used in the Document model, any TableData/type aliases, and
Store.updateTableAttrs signature to accept number | undefined | null as
appropriate) or add a normalization step that converts persisted nulls to
undefined before calling updateTableAttrs; update all call sites (including
td.rowHeights splice at the shown blockId path and the other occurrence around
lines 792-799) so the compiler recognizes and enforces nullish checks instead of
using casts.
Comment on lines
+667
to
+711
| private resolveTableFromMouse(e: MouseEvent): { | ||
| tableBlockId: string; | ||
| localX: number; | ||
| localY: number; | ||
| layout: import('./table-layout.js').LayoutTable; | ||
| tableOriginX: number; | ||
| tableOriginY: number; | ||
| } | null { | ||
| const rect = this.container.getBoundingClientRect(); | ||
| const s = this.getScaleFactor(); | ||
| const mouseX = (e.clientX - rect.left + this.container.scrollLeft) / s; | ||
| const mouseY = (e.clientY - rect.top - this.getCanvasOffsetTop()) / s + this.container.scrollTop / s; | ||
|
|
||
| const layout = this.getLayout(); | ||
| const paginatedLayout = this.getPaginatedLayout(); | ||
| const { margins } = paginatedLayout.pageSetup; | ||
| const pageX = getPageXOffset(paginatedLayout, this.getCanvasWidth()); | ||
|
|
||
| for (const lb of layout.blocks) { | ||
| if (lb.block.type !== 'table' || !lb.layoutTable) continue; | ||
| const tl = lb.layoutTable; | ||
| const blockIndex = layout.blocks.indexOf(lb); | ||
|
|
||
| let tablePageY = 0; | ||
| for (const page of paginatedLayout.pages) { | ||
| for (const pl of page.lines) { | ||
| if (pl.blockIndex === blockIndex && pl.lineIndex === 0) { | ||
| tablePageY = getPageYOffset(paginatedLayout, page.pageIndex) + pl.y; | ||
| break; | ||
| } | ||
| } | ||
| if (tablePageY !== 0) break; | ||
| } | ||
|
|
||
| const tableOriginX = pageX + margins.left; | ||
| const tableOriginY = tablePageY; | ||
| const localX = mouseX - tableOriginX; | ||
| const localY = mouseY - tableOriginY; | ||
|
|
||
| if (localX >= 0 && localX <= tl.totalWidth && localY >= 0 && localY <= tl.totalHeight) { | ||
| return { tableBlockId: lb.block.id, localX, localY, layout: tl, tableOriginX, tableOriginY }; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Use page-local Y coordinates for row-border hit-testing.
localY is anchored to the table’s first rendered line here. Once a table breaks across pages, the inter-page gap gets folded into that value while layout.rowYOffsets and totalHeight stay gap-free table coordinates, so hover/drag can miss the correct row border on later pages. Rebase Y against the page segment that actually contains the hovered rows, like the row-aware math in resolveOffsetInCellAtXY().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/src/view/text-editor.ts` around lines 667 - 711,
resolveTableFromMouse uses a tableOriginY anchored to the table's first rendered
line so localY includes inter-page gaps and can miss row-border hits on later
pages; change the Y rebasing to use the page-local segment that actually
contains the mouse similar to resolveOffsetInCellAtXY. Locate
resolveTableFromMouse, find where tablePageY/tableOriginY and localY are
computed, then determine which paginatedLayout.pages segment contains the mouse
Y (using the same row-aware math as in resolveOffsetInCellAtXY) and subtract
that page-segment origin when computing localY so comparisons against
tl.rowYOffsets and tl.totalHeight use gap-free table coordinates for
hit-testing.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/tasks/archive/2026/03/20260331-table-resize-todo.md (1)
895-895: Optional wording polish at Line 895.Replace “exactly what we want” with a less emphatic phrasing (e.g., “matches the intended behavior”) for cleaner technical tone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tasks/archive/2026/03/20260331-table-resize-todo.md` at line 895, Update the phrasing at the noted sentence to be less emphatic: replace “exactly what we want” with a more neutral phrase like “matches the intended behavior” (or similar) in the sentence that explains that reusing dragGuideline → paint() dashed line rendering from the ruler produces full-height/full-width dashed lines; no changes to dragGuideline or paint() code are required—only adjust the wording in the documentation line referencing the dashed line rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/tasks/archive/2026/03/20260331-table-resize-todo.md`:
- Line 15: The section heading "Task 1: Data Model — Add `rowHeights` to
`TableData`" currently uses an H3 (###) directly after top-level content which
triggers markdownlint MD001; change that heading to H2 (use "## Task 1: Data
Model — Add `rowHeights` to `TableData`") or insert an intermediate H2 before
the group of task sections so each task can remain H3, ensuring consistent
heading hierarchy across the document.
- Line 36: The doc uses a machine-specific absolute path in the run command ("cd
/Users/hackerwins/Development/wafflebase/wafflesheets && pnpm --filter
`@wafflebase/docs` test -- --run test/model/types.test.ts"); update all
occurrences (e.g., the command string present at the shown line and the other
listed lines) to use a repo-relative form such as "cd $PWD && pnpm --filter
`@wafflebase/docs` test -- --run test/model/types.test.ts" or simply "pnpm
--filter `@wafflebase/docs` test -- --run test/model/types.test.ts" so the
instruction runs from the repository root and is portable across machines.
---
Nitpick comments:
In `@docs/tasks/archive/2026/03/20260331-table-resize-todo.md`:
- Line 895: Update the phrasing at the noted sentence to be less emphatic:
replace “exactly what we want” with a more neutral phrase like “matches the
intended behavior” (or similar) in the sentence that explains that reusing
dragGuideline → paint() dashed line rendering from the ruler produces
full-height/full-width dashed lines; no changes to dragGuideline or paint() code
are required—only adjust the wording in the documentation line referencing the
dashed line rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b63d003-f220-4189-bfac-ea9c72ac18ac
📒 Files selected for processing (4)
docs/tasks/README.mddocs/tasks/archive/2026/03/20260331-table-resize-lessons.mddocs/tasks/archive/2026/03/20260331-table-resize-todo.mddocs/tasks/archive/README.md
✅ Files skipped from review due to trivial changes (3)
- docs/tasks/README.md
- docs/tasks/archive/README.md
- docs/tasks/archive/2026/03/20260331-table-resize-lessons.md
|
|
||
| --- | ||
|
|
||
| ### Task 1: Data Model — Add `rowHeights` to `TableData` |
There was a problem hiding this comment.
Fix heading level increment at Line 15.
Line 15 starts at ### immediately after top-level content; markdownlint MD001 is correct here. Use ## for task sections (or add an intermediate ## heading before them).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/tasks/archive/2026/03/20260331-table-resize-todo.md` at line 15, The
section heading "Task 1: Data Model — Add `rowHeights` to `TableData`" currently
uses an H3 (###) directly after top-level content which triggers markdownlint
MD001; change that heading to H2 (use "## Task 1: Data Model — Add `rowHeights`
to `TableData`") or insert an intermediate H2 before the group of task sections
so each task can remain H3, ensuring consistent heading hierarchy across the
document.
|
|
||
| - [x] **Step 2: Run test to verify it passes (baseline)** | ||
|
|
||
| Run: `cd /Users/hackerwins/Development/wafflebase/wafflesheets && pnpm --filter @wafflebase/docs test -- --run test/model/types.test.ts` |
There was a problem hiding this comment.
Use repo-relative commands instead of a machine-specific absolute path.
The repeated cd /Users/hackerwins/... makes the run steps non-portable for other contributors. Prefer commands that run from repo root (or use $PWD) so copy/paste works universally.
Suggested doc edit
-Run: `cd /Users/hackerwins/Development/wafflebase/wafflesheets && pnpm --filter `@wafflebase/docs` test -- --run test/model/types.test.ts`
+Run: `pnpm --filter `@wafflebase/docs` test -- --run test/model/types.test.ts`Also applies to: 54-54, 99-99, 124-124, 157-157, 222-222, 282-282, 307-307, 355-355, 376-376, 494-494, 608-608, 858-858, 899-899, 911-911
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/tasks/archive/2026/03/20260331-table-resize-todo.md` at line 36, The doc
uses a machine-specific absolute path in the run command ("cd
/Users/hackerwins/Development/wafflebase/wafflesheets && pnpm --filter
`@wafflebase/docs` test -- --run test/model/types.test.ts"); update all
occurrences (e.g., the command string present at the shown line and the other
listed lines) to use a repo-relative form such as "cd $PWD && pnpm --filter
`@wafflebase/docs` test -- --run test/model/types.test.ts" or simply "pnpm
--filter `@wafflebase/docs` test -- --run test/model/types.test.ts" so the
instruction runs from the repository root and is portable across machines.
- Critical: Serialize/deserialize rowHeights in Yorkie round-trip via buildBlockNode and treeNodeToBlock (comma-separated attribute) - Major: Widen rowHeights type to (number | undefined)[] across model, store, and Yorkie store — remove unsafe casts - Major: Add document-level mousemove/mouseup listeners during border drag so release outside the editor clears drag state - Minor: Guard against minPixel > maxPixel when adjacent columns are too narrow — collapse to no-op drag - Minor: Add language tags to fenced code blocks in design doc
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/docs/src/model/document.ts (1)
792-801: Consider adding bounds and value validation.The method writes to
td.rowHeights[rowIndex]without verifyingrowIndexis within[0, td.rows.length). If an invalid index is passed, it would write past the array bounds. Additionally, negative height values would be persisted (though the layout engine intable-layout.tswould ignore them).🛡️ Proposed fix with bounds check
setRowHeight(blockId: string, rowIndex: number, height: number): void { const block = this.getBlock(blockId); const td = block.tableData!; + if (rowIndex < 0 || rowIndex >= td.rows.length) { + return; // Invalid row index + } if (!td.rowHeights) { td.rowHeights = new Array(td.rows.length).fill(undefined); } td.rowHeights[rowIndex] = height; this.store.updateTableAttrs(blockId, { cols: td.columnWidths, rowHeights: td.rowHeights }); this.refresh(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/src/model/document.ts` around lines 792 - 801, The setRowHeight method currently writes td.rowHeights[rowIndex] without validating inputs; add checks in setRowHeight(blockId: string, rowIndex: number, height: number) to (1) ensure block and block.tableData (td) exist, (2) verify rowIndex is an integer within 0 <= rowIndex < td.rows.length (return or throw a clear error if out of range), and (3) validate height is a finite non-negative number (either clamp to 0 or reject invalid values). Also ensure td.rowHeights is initialized to length td.rows.length before assignment and only call this.store.updateTableAttrs(blockId, { cols: td.columnWidths, rowHeights: td.rowHeights }) and this.refresh() after validation/sanitization completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/docs/src/model/document.ts`:
- Around line 783-790: The resizeColumn method can write past the end of
td.columnWidths when colIndex is the last index; add a defensive bounds check in
resizeColumn (in class/method resizeColumn): retrieve block via getBlock and td
= block.tableData, then verify colIndex >= 0 and colIndex + 1 <
td.columnWidths.length (or clamp indices) and if invalid return early (no
updates or refresh), otherwise assign td.columnWidths[colIndex] and
td.columnWidths[colIndex + 1], call this.store.updateTableAttrs(blockId, { cols:
td.columnWidths }) and this.refresh(); this prevents sparse arrays/invalid
writes and keeps existing caller behavior intact.
---
Nitpick comments:
In `@packages/docs/src/model/document.ts`:
- Around line 792-801: The setRowHeight method currently writes
td.rowHeights[rowIndex] without validating inputs; add checks in
setRowHeight(blockId: string, rowIndex: number, height: number) to (1) ensure
block and block.tableData (td) exist, (2) verify rowIndex is an integer within 0
<= rowIndex < td.rows.length (return or throw a clear error if out of range),
and (3) validate height is a finite non-negative number (either clamp to 0 or
reject invalid values). Also ensure td.rowHeights is initialized to length
td.rows.length before assignment and only call
this.store.updateTableAttrs(blockId, { cols: td.columnWidths, rowHeights:
td.rowHeights }) and this.refresh() after validation/sanitization completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6e103d2-94bf-4273-b12e-53ca3f9962be
📒 Files selected for processing (9)
docs/design/docs-table-resize.mdpackages/docs/src/model/document.tspackages/docs/src/model/types.tspackages/docs/src/store/memory.tspackages/docs/src/store/store.tspackages/docs/src/view/table-resize.tspackages/docs/src/view/text-editor.tspackages/docs/test/view/table-layout.test.tspackages/frontend/src/app/docs/yorkie-doc-store.ts
✅ Files skipped from review due to trivial changes (1)
- packages/docs/src/model/types.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/docs/src/store/store.ts
- packages/docs/src/store/memory.ts
- packages/docs/test/view/table-layout.test.ts
- packages/frontend/src/app/docs/yorkie-doc-store.ts
- docs/design/docs-table-resize.md
- packages/docs/src/view/table-resize.ts
- packages/docs/src/view/text-editor.ts
Comment on lines
+783
to
+790
| resizeColumn(blockId: string, colIndex: number, leftRatio: number, rightRatio: number): void { | ||
| const block = this.getBlock(blockId); | ||
| const td = block.tableData!; | ||
| td.columnWidths[colIndex] = leftRatio; | ||
| td.columnWidths[colIndex + 1] = rightRatio; | ||
| this.store.updateTableAttrs(blockId, { cols: td.columnWidths }); | ||
| this.refresh(); | ||
| } |
There was a problem hiding this comment.
Missing bounds check for colIndex + 1 could corrupt table state.
If colIndex equals td.columnWidths.length - 1 (the last column), writing to td.columnWidths[colIndex + 1] creates a sparse array or writes to an invalid index. While the caller in text-editor.ts may prevent this, a defensive check here would prevent silent corruption.
🛡️ Proposed fix with bounds check and early return
resizeColumn(blockId: string, colIndex: number, leftRatio: number, rightRatio: number): void {
const block = this.getBlock(blockId);
const td = block.tableData!;
+ if (colIndex < 0 || colIndex + 1 >= td.columnWidths.length) {
+ return; // Cannot resize: no adjacent column to the right
+ }
td.columnWidths[colIndex] = leftRatio;
td.columnWidths[colIndex + 1] = rightRatio;
this.store.updateTableAttrs(blockId, { cols: td.columnWidths });
this.refresh();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resizeColumn(blockId: string, colIndex: number, leftRatio: number, rightRatio: number): void { | |
| const block = this.getBlock(blockId); | |
| const td = block.tableData!; | |
| td.columnWidths[colIndex] = leftRatio; | |
| td.columnWidths[colIndex + 1] = rightRatio; | |
| this.store.updateTableAttrs(blockId, { cols: td.columnWidths }); | |
| this.refresh(); | |
| } | |
| resizeColumn(blockId: string, colIndex: number, leftRatio: number, rightRatio: number): void { | |
| const block = this.getBlock(blockId); | |
| const td = block.tableData!; | |
| if (colIndex < 0 || colIndex + 1 >= td.columnWidths.length) { | |
| return; // Cannot resize: no adjacent column to the right | |
| } | |
| td.columnWidths[colIndex] = leftRatio; | |
| td.columnWidths[colIndex + 1] = rightRatio; | |
| this.store.updateTableAttrs(blockId, { cols: td.columnWidths }); | |
| this.refresh(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/src/model/document.ts` around lines 783 - 790, The resizeColumn
method can write past the end of td.columnWidths when colIndex is the last
index; add a defensive bounds check in resizeColumn (in class/method
resizeColumn): retrieve block via getBlock and td = block.tableData, then verify
colIndex >= 0 and colIndex + 1 < td.columnWidths.length (or clamp indices) and
if invalid return early (no updates or refresh), otherwise assign
td.columnWidths[colIndex] and td.columnWidths[colIndex + 1], call
this.store.updateTableAttrs(blockId, { cols: td.columnWidths }) and
this.refresh(); this prevents sparse arrays/invalid writes and keeps existing
caller behavior intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
col-resize/row-resizecursor, drag with guideline overlay, apply on mouse-uprowHeightsfield onTableDataChanges
rowHeights?: number[]toTableData, sync on row insert/deleteresizeColumn()(adjacent-only) andsetRowHeight()methodscomputeTableLayout()table-resize.tsmodule withdetectTableBorder()andcreateDragState()dragGuidelinemechanismupdateTableAttrs, fix canvas cursor targeting withdata-roleTest plan
resizeColumn(),setRowHeight(), row insert/delete synccomputeTableLayout()with user row heightsdetectTableBorder()(9 cases: borders, edges, intersections)pnpm verify:fastpasses (297 tests)col-resizecursorrow-resizecursorDesign doc:
docs/design/docs-table-resize.md🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests