From ad38ac233bb7d885b707ebe12388775b1e511233 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Tue, 5 Sep 2023 09:27:12 +0100 Subject: [PATCH] Merge pull request #14653 from overleaf/mj-column-spacing-parsing [visual] Support cell spacing declarations GitOrigin-RevId: 16b4ddc196558679301010378912b14f6295e05f --- .../components/table-generator/tabular.tsx | 2 + .../table-generator/toolbar/commands.ts | 55 ++++++++++--------- .../components/table-generator/utils.ts | 50 +++++++++++++++-- .../visual/visual-widgets/tabular.tsx | 1 + ...codemirror-editor-table-generator.spec.tsx | 23 ++++++++ 5 files changed, 101 insertions(+), 30 deletions(-) diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx b/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx index 7cc93022b3..b0e9fe2d97 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx +++ b/services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx @@ -29,6 +29,8 @@ export type ColumnDefinition = { borderLeft: number borderRight: number content: string + cellSpacingLeft: string + cellSpacingRight: string } export type CellData = { diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/toolbar/commands.ts b/services/web/frontend/js/features/source-editor/components/table-generator/toolbar/commands.ts index 7163c740f2..bdcaefbb0d 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/toolbar/commands.ts +++ b/services/web/frontend/js/features/source-editor/components/table-generator/toolbar/commands.ts @@ -71,7 +71,9 @@ export const setBorders = ( ], }) } else if (theme === BorderTheme.FULLY_BORDERED) { - const newSpec = addColumnBordersToSpecification(specification) + const newSpec = generateColumnSpecification( + addColumnBordersToSpecification(table.columns) + ) const insertColumns = view.state.changes({ from: positions.columnDeclarations.from, @@ -110,14 +112,14 @@ export const setBorders = ( for (const row of table.rows) { for (const cell of row.cells) { if (cell.multiColumn) { - const specification = view.state.sliceDoc( - cell.multiColumn.columns.from, - cell.multiColumn.columns.to - ) addMulticolumnBorders.push({ from: cell.multiColumn.columns.from, to: cell.multiColumn.columns.to, - insert: addColumnBordersToSpecification(specification), + insert: generateColumnSpecification( + addColumnBordersToSpecification( + cell.multiColumn.columns.specification + ) + ), }) } } @@ -129,24 +131,13 @@ export const setBorders = ( } } -const addColumnBordersToSpecification = (specification: string) => { - let newSpec = '|' - let consumingBrackets = 0 - for (const char of specification) { - if (char === '{') { - consumingBrackets++ - } - if (char === '}' && consumingBrackets > 0) { - consumingBrackets-- - } - if (consumingBrackets) { - newSpec += char - } - if (char === '|') { - continue - } - newSpec += char + '|' - } +const addColumnBordersToSpecification = (specification: ColumnDefinition[]) => { + const newSpec = specification.map(column => ({ + ...column, + borderLeft: 1, + borderRight: 0, + })) + newSpec[newSpec.length - 1].borderRight = 1 return newSpec } @@ -216,8 +207,18 @@ export const setAlignment = ( const generateColumnSpecification = (columns: ColumnDefinition[]) => { return columns .map( - ({ borderLeft, borderRight, content }) => - `${'|'.repeat(borderLeft)}${content}${'|'.repeat(borderRight)}` + ({ + borderLeft, + borderRight, + content, + cellSpacingLeft, + cellSpacingRight, + }) => + `${'|'.repeat( + borderLeft + )}${cellSpacingLeft}${content}${cellSpacingRight}${'|'.repeat( + borderRight + )}` ) .join('') } @@ -427,6 +428,8 @@ export const insertColumn = ( borderLeft: 0, borderRight, content: 'l', + cellSpacingLeft: '', + cellSpacingRight: '', })) ) if (targetIndex === 0 && borderTheme === BorderTheme.FULLY_BORDERED) { diff --git a/services/web/frontend/js/features/source-editor/components/table-generator/utils.ts b/services/web/frontend/js/features/source-editor/components/table-generator/utils.ts index 48fb3c906d..65a9fe0bb4 100644 --- a/services/web/frontend/js/features/source-editor/components/table-generator/utils.ts +++ b/services/web/frontend/js/features/source-editor/components/table-generator/utils.ts @@ -12,6 +12,24 @@ export type RowPosition = { hlines: { from: number; to: number }[] } +function parseArgument(spec: string, startIndex: number): number { + if (spec.charAt(startIndex) !== '{') { + throw new Error('Missing opening brace') + } + let depth = 0 + for (let i = startIndex; i < spec.length; i++) { + if (spec.charAt(i) === '{') { + depth++ + } else if (spec.charAt(i) === '}') { + depth-- + } + if (depth === 0) { + return i + } + } + throw new Error('Missing closing brace') +} + export function parseColumnSpecifications( specification: string ): ColumnDefinition[] { @@ -20,6 +38,8 @@ export function parseColumnSpecifications( let currentBorderLeft = 0 let currentBorderRight = 0 let currentContent = '' + let currentCellSpacingLeft = '' + let currentCellSpacingRight = '' function maybeCommit() { if (currentAlignment !== undefined) { columns.push({ @@ -27,11 +47,15 @@ export function parseColumnSpecifications( borderLeft: currentBorderLeft, borderRight: currentBorderRight, content: currentContent, + cellSpacingLeft: currentCellSpacingLeft, + cellSpacingRight: currentCellSpacingRight, }) currentAlignment = undefined currentBorderLeft = 0 currentBorderRight = 0 currentContent = '' + currentCellSpacingLeft = '' + currentCellSpacingRight = '' } } for (let i = 0; i < specification.length; i++) { @@ -65,12 +89,31 @@ export function parseColumnSpecifications( currentAlignment = 'paragraph' currentContent += 'p' // TODO: Parse these details - while (i < specification.length && specification.charAt(i) !== '}') { - i++ - currentContent += specification.charAt(i) + const argumentEnd = parseArgument(specification, i + 1) + // Don't include the p twice + currentContent += specification.slice(i + 1, argumentEnd + 1) + i = argumentEnd + break + } + case '@': + case '!': { + const argumentEnd = parseArgument(specification, i + 1) + // Include the @/! + const argument = specification.slice(i, argumentEnd + 1) + i = argumentEnd + if (currentAlignment) { + // We have a cell, so this is right cell spacing + currentCellSpacingRight = argument + } else { + currentCellSpacingLeft = argument } break } + case ' ': + case '\n': + case '\t': + currentContent += char + break } } maybeCommit() @@ -279,7 +322,6 @@ function parseTabularBody( } else if (isHLine(currentChild)) { const lastCell = getLastCell() if (lastCell?.content.trim()) { - console.error(lastCell) throw new Error('\\hline must be at the start of a row') } // push start of cell past the hline diff --git a/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/tabular.tsx b/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/tabular.tsx index 2bb4953c8a..b4f95acf4d 100644 --- a/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/tabular.tsx +++ b/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/tabular.tsx @@ -22,6 +22,7 @@ export class TabularWidget extends WidgetType { try { this.parseResult = generateTable(tabularNode, state) } catch (e) { + console.error(e) this.parseResult = null } } diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx index 43aed8dd3e..45f07a045e 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx +++ b/services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx @@ -422,5 +422,28 @@ cell 3 & cell 4 \\\\ cy.get('.ol-cm-command-caption').should('not.exist') cy.get('.ol-cm-command-label').should('not.exist') }) + + it('Renders a table with custom column spacing', function () { + mountEditor(` +\\begin{tabular}{@{}c@{}l!{}} + cell 1 & cell 2 \\\\ + cell 3 & cell 4 \\\\ +\\end{tabular}`) + checkTable([ + ['cell 1', 'cell 2'], + ['cell 3', 'cell 4'], + ]) + cy.get('.table-generator-cell').first().click() + cy.get('.table-generator-floating-toolbar').as('toolbar').should('exist') + cy.get('@toolbar').findByText('No borders').click() + cy.get('.table-generator').findByText('All borders').click() + // The element is partially covered, but we can still click it + cy.get('.cm-line').first().click({ force: true }) + checkTable([ + ['cell 1', 'cell 2'], + ['cell 3', 'cell 4'], + ]) + checkBordersWithNoMultiColumn([true, true, true], [true, true, true]) + }) }) })