fix: table toolbar, header columns, click-to-select, wrap and auto-fit (#556) - #557
Open
bvnaik05 wants to merge 6 commits into
Open
fix: table toolbar, header columns, click-to-select, wrap and auto-fit (#556)#557bvnaik05 wants to merge 6 commits into
bvnaik05 wants to merge 6 commits into
Conversation
…ct cells (frappe#556) Combines the separate Edit Table / Table Actions controls into one icon-only Table menu, unifies table- and cell-level text colour into a single Text Colour control, adds Fill and Border to match Shape formatting, and moves Delete to the end of the toolbar. Adds header columns alongside header rows, independently configurable. A single click on a picked table now selects a cell instead of opening it for editing — only a double click opens it — with click+drag range selection and click-outside deselection unchanged. Adds a font selector for table cell text, reusing the shared Espresso font list now extracted into diagram/textFonts.js. Auto-fit on double-clicking a resize handle and cell text wrapping (no scrollbars, row auto-grow) are not yet in this PR — tracked as the remaining slices of frappe#556. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…#556) Lint: canMerge/canSplit in WhiteboardTable.vue and extended in startCellRangeDrag were dead once showRange stopped reading them and the click-vs-drag release branch was removed. E2E: table-cell-text.spec.js opened a cell with two separate clicks (the old T2 path); a plain click now only selects, so opening a cell needs a double click. Also repoints the cell-colour test at the unified Text Colour control ("Cell text colour" was folded into it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… auto-fit (frappe#556) A cell now wraps its text instead of scrolling horizontally, and its row grows live as it's typed into — mirroring WhiteboardStickyNote's growToText/commit split (unrecorded growth while typing, the final size landing with the text in one undo step). Enter inserts a line break instead of committing, reusing the same newlineIntent list-continuation the sticky note uses; commit now happens on click-away, matching how a sticky note behaves. Adds a mark-aware run wrapper (wrapRuns, richText.js) so a wrapped cell keeps its bold/italic/underline/strike marks on the exact characters they belonged to — including the original whitespace's own marks, not a reconstructed space that could silently move a formatting boundary. The committed SVG render and the export mirror the same wrapped, marked lines. Double-clicking a column or row's resize handle auto-fits that dimension to its content, reusing the same undo labels a manual drag already uses. lineBeforeCaret/deleteBeforeCaret move from WhiteboardStickyNote.vue into richTextDom.js so the table cell editor can reuse them rather than holding a second copy. Known gap, not introduced by this change: clicking empty canvas does not close/deselect an open table on a UNIFIED document (most new diagrams) — the select tool's empty-click path there (useSelection.js) never reaches the whiteboard UI's own clearSelection, which is what actually clears editingCell/ cellRange. Reproduces identically on main before this branch's changes, so it predates frappe#556 entirely. Filing separately rather than risking a rushed fix to shared click-dispatch code shared by every whiteboard object type, not just tables. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bvnaik05
changed the title
…#556) Enter now inserts a line break in a wrapping cell instead of closing it, so table-cell-text.spec.js's commit step (previously page.keyboard.press('Enter')) never landed. Replaced with a click on the seeded table's neighbouring cell, which reliably commits (startCellRangeDrag nulls editingCell on every press, whichever cell it lands on) without depending on the separate, currently- broken empty-canvas deselect path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
frappe#556) boxInWindow(page, cell, ...) returns the "CELL-TEXT" <text> element's own rendered bounding box — as wide as the glyphs (~60-90px) — not the table cell's (120-160px in these fixtures). A 1.5x multiple of that box never left the originating cell, so the click landed on the still-open editor's own div (pointerdown.stop) instead of the table underneath, and nothing committed. Fixed to a flat 200px offset from the text's left edge, which clears both fixtures' cell widths into the neighbouring column. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…up (frappe#556) Feedback on the combined Table control: - Its icon (lucide-table) was the same one the Table INSERT tool and the toolbar's Table trigger area already use — swapped to lucide-table-properties so it reads as "settings", not "insert" or "the Merge icon's neighbour". - Moved it next to Merge/Split by rendering TableCellGroup before WhiteboardObjectGroup in CanvasToolbar.vue, so the cell group's trailing Merge/Split sits immediately before the object group's leading Table control — both reshape the table, just gated differently. Delete stays the true last item, at the end of the object group either way. - The popup ran off the bottom of the screen: a single narrow (176px) column stacked Rows/Columns/header checkboxes/alignment/the action menu one item per line. Widened to 320px (w-72, matching TableOptions) and laid Rows + Columns, the two header checkboxes, and the action menu's own groups out side by side, roughly halving the popup's height. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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
All 18 items of #556 except one known pre-existing gap (see below):
Known gap — not introduced by this branch
Clicking empty canvas does not close/deselect an open table on a unified document (the type "Create" makes, i.e. most new diagrams). Root cause:
useSelection.js's empty-click path (the shared block-shape selection) never reaches the whiteboard UI's ownclearSelection, which is what actually clearseditingCell/cellRange— a gap in the unified-canvas click-dispatch architecture shared by every whiteboard object type (stickies, lines, strokes), not just tables. Reproduces identically onmainbefore this branch, confirmed by stashing this branch's commits and rebuilding. Left unfixed here rather than risk a rushed change to click dispatch shared across the whole canvas — happy to take it as a fast follow-up if wanted.Everything else in #556's acceptance criteria is implemented and covered by tests below.
Test plan
yarn vitest run— 1765 tests passingyarn build/yarn lint— clean🤖 Generated with Claude Code