From 3f9e68f899bf29e2199b7b3721e025c363ce3f27 Mon Sep 17 00:00:00 2001 From: Bhargavi Naik Date: Wed, 19 Aug 2026 11:15:23 +0000 Subject: [PATCH 1/7] fix: consolidate the table toolbar, header columns, and click-to-select cells (#556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #556. Co-Authored-By: Claude Sonnet 5 --- .../src/components/canvas/WhiteboardTable.vue | 56 ++-- .../src/components/floating/TableOptions.vue | 12 +- .../toolbar/groups/TableCellGroup.vue | 82 ++---- .../components/toolbar/groups/TextGroup.vue | 25 +- .../toolbar/groups/WhiteboardObjectGroup.vue | 246 +++++++++++++++--- .../components/toolbar/groups/tableMenu.js | 13 +- .../toolbar/groups/tableMenu.test.js | 7 + .../src/components/toolbar/textGroup.test.js | 15 +- .../src/composables/useTableCellFormat.js | 16 +- frontend/src/composables/useTableSelection.js | 14 +- .../src/composables/useTableSelection.test.js | 17 +- frontend/src/composables/useThumbnail.js | 18 +- .../composables/useWhiteboardInteraction.js | 30 ++- .../useWhiteboardInteraction.test.js | 39 ++- frontend/src/diagram/tableStructure.js | 28 ++ frontend/src/diagram/tableStructure.test.js | 36 +++ frontend/src/diagram/textFonts.js | 24 ++ frontend/src/diagram/whiteboardModel.js | 22 +- frontend/src/diagram/whiteboardModel.test.js | 38 ++- frontend/src/stores/useDiagramStore.js | 6 + .../src/stores/useDiagramStore.table.test.js | 20 +- 21 files changed, 603 insertions(+), 161 deletions(-) create mode 100644 frontend/src/diagram/textFonts.js diff --git a/frontend/src/components/canvas/WhiteboardTable.vue b/frontend/src/components/canvas/WhiteboardTable.vue index c78d04cc..64c846bb 100644 --- a/frontend/src/components/canvas/WhiteboardTable.vue +++ b/frontend/src/components/canvas/WhiteboardTable.vue @@ -1,9 +1,10 @@ diff --git a/frontend/src/components/toolbar/groups/TextGroup.vue b/frontend/src/components/toolbar/groups/TextGroup.vue index 8ec4fb04..c81e0018 100644 --- a/frontend/src/components/toolbar/groups/TextGroup.vue +++ b/frontend/src/components/toolbar/groups/TextGroup.vue @@ -13,6 +13,7 @@ import { useBlockSelection } from '@/composables/useBlockSelection.js' import { richCommands, isMarkActive } from '@/composables/useRichText.js' import EspressoSwatchGrid from '@/components/palette-right/EspressoSwatchGrid.vue' import ToolbarButton from '../ToolbarButton.vue' +import { ESPRESSO_SANS, FONTS } from '@/diagram/textFonts.js' // textShapes, not every selected shape (#519): a mixed selection of an image and a // rectangle reads and writes the rectangle's label, and passes the image by. Reading @@ -21,30 +22,6 @@ import ToolbarButton from '../ToolbarButton.vue' const { store, textShapes, editing } = useBlockSelection() const textIds = computed(() => textShapes.value.map((shape) => shape.id)) -// Espresso defines exactly two typefaces (design/colors_and_type.css): the Inter -// sans stack and a mono stack. Those two now match it character for character (#475). -// -// Inter used to be `value: ''`, which inherited whatever the canvas happened to be -// set in rather than naming a stack. Mono was Espresso's list minus 'JetBrains Mono'. -// -// Serif and Handwritten have no Espresso equivalent and stay as canvas-only extras — -// CLAUDE.md cardinal rule 2 makes the SVG canvas the explicit exception to chrome -// tokens, so the canvas is allowed a look of its own. -// -// "Rounded" is gone. Its stack asked for Nunito, which was never loaded anywhere in -// the app — no @font-face, no import — so it fell back to Segoe UI on Windows and -// plain system-ui on macOS and had never once rendered as anything rounded. An -// option that does nothing is worse than one fewer option, and shipping a webfont to -// rescue it was not worth the download. -const ESPRESSO_SANS = "'Inter', 'Inter Variable', system-ui, -apple-system, 'Segoe UI', sans-serif" -const ESPRESSO_MONO = "ui-monospace, 'JetBrains Mono', 'SF Mono', Menlo, monospace" - -const FONTS = [ - { label: 'Inter', value: ESPRESSO_SANS }, - { label: 'Serif', value: 'Georgia, "Times New Roman", serif' }, - { label: 'Mono', value: ESPRESSO_MONO }, - { label: 'Handwritten', value: "'Bradley Hand', 'Chalkboard SE', 'Comic Sans MS', 'Segoe Print', cursive" }, -] const MARKS = [ { name: 'bold', label: 'Bold', icon: 'lucide-bold' }, { name: 'italic', label: 'Italic', icon: 'lucide-italic' }, diff --git a/frontend/src/components/toolbar/groups/WhiteboardObjectGroup.vue b/frontend/src/components/toolbar/groups/WhiteboardObjectGroup.vue index 642159bd..3f67b9a9 100644 --- a/frontend/src/components/toolbar/groups/WhiteboardObjectGroup.vue +++ b/frontend/src/components/toolbar/groups/WhiteboardObjectGroup.vue @@ -5,22 +5,40 @@ // That Delete is the only one a line, table or stroke has by mouse, which is why // the bar it replaces had to mount on a unified document as well as a whiteboard // one. A lone sticky is handled by its own richer group, so it is skipped here. +// +// A table stays selected for the whole lifetime of a cell/range pick (#553): +// startCellRangeDrag never reselects, it only sets cellRange/editingCell. So this +// group is the one place that has to work in BOTH "table selected, no cell +// picked" and "table selected, cell/range picked" modes — the Table menu, Text +// Colour, Fill and Border all read useTableSelection().hasSelection to pick which +// one they are reading/writing (#556). TableCellGroup, by contrast, only ever +// shows once a cell IS picked, so it stays cell-only. import { computed } from 'vue' -import { Popover } from 'frappe-ui' +import { Popover, TextInput, ItemListRow } from 'frappe-ui' import { useDiagramStore } from '@/stores/useDiagramStore.js' import { useWhiteboardUi } from '@/composables/useWhiteboardUi.js' -import { lineById, tableById } from '@/diagram/whiteboardModel.js' -import { tableHeaderRows } from '@/diagram/tableStructure.js' +import { useTableSelection } from '@/composables/useTableSelection.js' +import { lineById, tableById, tableCellStyle } from '@/diagram/whiteboardModel.js' +import { tableHeaderRows, tableHeaderCols } from '@/diagram/tableStructure.js' +import { tableMenuOptions } from './tableMenu.js' import LineOptions from '@/components/floating/LineOptions.vue' import TableOptions from '@/components/floating/TableOptions.vue' +import ColorPicker from '@/components/palette-right/ColorPicker.vue' import EspressoSwatchGrid from '@/components/palette-right/EspressoSwatchGrid.vue' import ToolbarButton from '../ToolbarButton.vue' +import ToolbarSeparator from '../ToolbarSeparator.vue' + +// Border styles shown VISUALLY (a line preview), matching FillBorderSection's +// shape-border control — the same visual language, on a table (#556). +const DASH_STYLES = ['solid', 'dashed', 'dotted'] +const DASH_ARRAY = { solid: '0', dashed: '5 3', dotted: '1.5 3' } const store = useDiagramStore() const ui = useWhiteboardUi() +const selection = useTableSelection() -const selection = computed(() => ui.state.selection || []) -const multi = computed(() => selection.value.length > 1) +const selectionList = computed(() => ui.state.selection || []) +const multi = computed(() => selectionList.value.length > 1) const selected = computed(() => ui.state.selected) const kind = computed(() => selected.value?.kind) @@ -32,77 +50,245 @@ const table = computed(() => ) const show = computed(() => multi.value || Boolean(selected.value && kind.value !== 'sticky')) -// A table's colour is its TEXT colour (#553) — the grid and the header band are -// neutral chrome — so it is set from the same "A" control a text box carries, -// rather than from a drawing palette buried in the table's options. -const tableColor = computed(() => table.value?.color || '#171717') - -// The header checkbox now writes a header ROW COUNT, and that has to travel -// through the model's own writer so the legacy `hasHeader` flag stays in step. +// The header checkboxes write a header ROW or COLUMN count, and those have to +// travel through the model's own writers — the row one keeps the legacy +// `hasHeader` flag in step, the column one (#556) has no legacy shape to keep. function changeTable(patch) { if ('headerRows' in patch) store.setTableHeaderRows(table.value.id, patch.headerRows) + else if ('headerCols' in patch) store.setTableHeaderCols(table.value.id, patch.headerCols) else store.updateTable(table.value.id, patch) } +// Combined "Edit table" + "Table actions" into one control (#556): the popup +// holds the row/column steppers and header checkboxes (TableOptions) followed by +// the same structural actions the old separate dropdown offered. Rendered as +// plain ItemListRows rather than through frappe-ui's Dropdown/Menu — a Dropdown +// item is a Reka menu row that treats any click inside it as "select the item", +// which would fight TableOptions' own steppers and checkboxes sitting in the +// same popup; ItemListRow is the same row frappe-ui's own Menu renders with, +// used directly, with none of that selection wiring. +// tableMenuOptions' delete/header-toggle entries all act on a target row or +// column, which only exists once a cell is picked (selection.hasSelection). With +// nothing picked — reachable now that the combined control shows for the table +// alone (#556) — every one of those actions would be a silent no-op (onSelection +// guards on `bounds`, and `rows.value.every(...)` on an empty array is +// vacuously true, which used to read as "already the header" for a picked +// selection but would misread as true here too). So this state gets its own +// smaller menu: only inserts, which have an obvious top/bottom/left/right +// target without a pick, plus deleting the table itself. +const tableMenu = computed(() => { + if (!table.value) return [] + if (selection.hasSelection.value) { + return tableMenuOptions({ + rowCount: selection.rows.value.length, + columnCount: selection.columns.value.length, + isHeader: selection.selectionIsHeader.value, + isHeaderColumn: selection.selectionIsHeaderColumn.value, + actions: selection, + }) + } + const id = table.value.id + return [ + { + group: 'Rows', + key: 'rows', + options: [ + { label: 'Insert row above', icon: 'lucide-arrow-up', onClick: () => store.insertTableRow(id, 0) }, + { label: 'Insert row below', icon: 'lucide-arrow-down', onClick: () => store.insertTableRow(id, table.value.rows) }, + ], + }, + { + group: 'Columns', + key: 'columns', + options: [ + { label: 'Insert column left', icon: 'lucide-arrow-left', onClick: () => store.insertTableColumn(id, 0) }, + { label: 'Insert column right', icon: 'lucide-arrow-right', onClick: () => store.insertTableColumn(id, table.value.cols) }, + ], + }, + { + group: 'Table', + key: 'table', + options: [{ label: 'Delete table', icon: 'lucide-trash-2', onClick: () => selection.deleteTable() }], + }, + ] +}) + +// ----- text colour, fill, border: one control each, working on whichever of +// "the table" or "the picked cell(s)" is current (#556). ----- + +const firstCell = computed(() => selection.cells.value[0] || null) +const cellStyle = computed(() => + firstCell.value ? tableCellStyle(table.value, firstCell.value.row, firstCell.value.col) : null, +) + +function writeCellsOrTable(cellPatch, tablePatch) { + if (!table.value) return + if (selection.hasSelection.value) store.setTableCellStyle(table.value.id, selection.cells.value, cellPatch) + else store.updateTable(table.value.id, tablePatch) +} + +const textColor = computed(() => + selection.hasSelection.value ? cellStyle.value?.color || '#171717' : table.value?.color || '#171717', +) +function setTextColor(hex) { + writeCellsOrTable({ color: hex }, { color: hex }) +} + +const fillColor = computed(() => + selection.hasSelection.value ? cellStyle.value?.fill : table.value?.fill, +) +function setFill(hex) { + writeCellsOrTable({ fill: hex }, { fill: hex }) +} + +const border = computed(() => + selection.hasSelection.value + ? cellStyle.value?.border + : { color: table.value?.border?.color || '#171717', width: table.value?.border?.width ?? 1, dash: table.value?.border?.dash || 'solid' }, +) +function setBorderColor(hex) { + writeCellsOrTable({ borderColor: hex }, { border: { color: hex } }) +} +function setBorderWidth(value) { + const width = Number(value) + if (width >= 0) writeCellsOrTable({ borderWidth: width }, { border: { width } }) +} +function setBorderDash(value) { + writeCellsOrTable({ borderDash: value }, { border: { dash: value } }) +} + function remove() { - store.removeWhiteboardObjects([...selection.value]) + store.removeWhiteboardObjects([...selectionList.value]) ui.clearSelection() }