diff --git a/packages/roosterjs-content-model-api/lib/publicApi/table/applyTableBorderFormat.ts b/packages/roosterjs-content-model-api/lib/publicApi/table/applyTableBorderFormat.ts index 0bf8f6353295..0c481c415eb5 100644 --- a/packages/roosterjs-content-model-api/lib/publicApi/table/applyTableBorderFormat.ts +++ b/packages/roosterjs-content-model-api/lib/publicApi/table/applyTableBorderFormat.ts @@ -5,6 +5,7 @@ import { mutateBlock, getTableMetadata, parseValueWithUnit, + parseColor, setFirstColumnFormatBorders, updateTableCellMetadata, } from 'roosterjs-content-model-dom'; @@ -23,6 +24,15 @@ import type { */ type BorderPositions = 'borderTop' | 'borderBottom' | 'borderLeft' | 'borderRight'; +/** + * @internal + * Border positions to update on a cell + */ +type BorderUpdate = { + cell: ReadonlyContentModelTableCell; + positions: BorderPositions[]; +}; + /** * @internal * Perimeter of the table selection @@ -37,6 +47,7 @@ type Perimeter = { /** * Operations to apply border + * Remove the targeted borders instead if they all already match the requested format. * @param editor The editor instance * @param border The border to apply * @param operation The operation to apply @@ -94,6 +105,13 @@ export function applyTableBorderFormat( const isRtl = tableModel.format.direction == 'rtl'; if (sel) { + const borderUpdates: BorderUpdate[] = []; + const collectBorderFormat = ( + cell: ReadonlyContentModelTableCell, + positions: BorderPositions[] + ) => { + borderUpdates.push({ cell, positions }); + }; const operations: BorderOperations[] = [operation]; while (operations.length) { switch (operations.pop()) { @@ -121,7 +139,7 @@ export function applyTableBorderFormat( ) { const cell = tableModel.rows[rowIndex].cells[colIndex]; // Format cells - All borders - applyBorderFormat(cell, borderFormat, allBorders); + collectBorderFormat(cell, allBorders); } } @@ -143,7 +161,7 @@ export function applyTableBorderFormat( isRtl ? sel.lastColumn : sel.firstColumn ]; // Format cells - Left border - applyBorderFormat(cell, borderFormat, leftBorder); + collectBorderFormat(cell, leftBorder); } // Format perimeter @@ -161,7 +179,7 @@ export function applyTableBorderFormat( isRtl ? sel.firstColumn : sel.lastColumn ]; // Format cells - Right border - applyBorderFormat(cell, borderFormat, rightBorder); + collectBorderFormat(cell, rightBorder); } // Format perimeter @@ -176,7 +194,7 @@ export function applyTableBorderFormat( ) { const cell = tableModel.rows[sel.firstRow].cells[colIndex]; // Format cells - Top border - applyBorderFormat(cell, borderFormat, topBorder); + collectBorderFormat(cell, topBorder); } // Format perimeter @@ -191,7 +209,7 @@ export function applyTableBorderFormat( ) { const cell = tableModel.rows[sel.lastRow].cells[colIndex]; // Format cells - Bottom border - applyBorderFormat(cell, borderFormat, bottomBorder); + collectBorderFormat(cell, bottomBorder); } // Format perimeter @@ -207,9 +225,8 @@ export function applyTableBorderFormat( } // Single column selection if (singleCol) { - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.firstRow].cells[sel.firstColumn], - borderFormat, ['borderBottom'] ); for ( @@ -219,25 +236,20 @@ export function applyTableBorderFormat( ) { const cell = tableModel.rows[rowIndex].cells[sel.firstColumn]; - applyBorderFormat(cell, borderFormat, [ - 'borderTop', - 'borderBottom', - ]); + collectBorderFormat(cell, ['borderTop', 'borderBottom']); } - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.lastRow].cells[sel.firstColumn], - borderFormat, ['borderTop'] ); break; } // Single row selection if (singleRow) { - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.firstRow].cells[ isRtl ? sel.lastColumn : sel.firstColumn ], - borderFormat, ['borderRight'] ); for ( @@ -246,16 +258,12 @@ export function applyTableBorderFormat( colIndex++ ) { const cell = tableModel.rows[sel.firstRow].cells[colIndex]; - applyBorderFormat(cell, borderFormat, [ - 'borderLeft', - 'borderRight', - ]); + collectBorderFormat(cell, ['borderLeft', 'borderRight']); } - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.firstRow].cells[ isRtl ? sel.firstColumn : sel.lastColumn ], - borderFormat, ['borderLeft'] ); break; @@ -263,35 +271,31 @@ export function applyTableBorderFormat( // For multiple rows and columns selections // Top left cell - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.firstRow].cells[ isRtl ? sel.lastColumn : sel.firstColumn ], - borderFormat, ['borderBottom', 'borderRight'] ); // Top right cell - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.firstRow].cells[ isRtl ? sel.firstColumn : sel.lastColumn ], - borderFormat, ['borderBottom', 'borderLeft'] ); // Bottom left cell - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.lastRow].cells[ isRtl ? sel.lastColumn : sel.firstColumn ], - borderFormat, ['borderTop', 'borderRight'] ); // Bottom right cell - applyBorderFormat( + collectBorderFormat( tableModel.rows[sel.lastRow].cells[ isRtl ? sel.firstColumn : sel.lastColumn ], - borderFormat, ['borderTop', 'borderLeft'] ); // First row @@ -301,7 +305,7 @@ export function applyTableBorderFormat( colIndex++ ) { const cell = tableModel.rows[sel.firstRow].cells[colIndex]; - applyBorderFormat(cell, borderFormat, [ + collectBorderFormat(cell, [ 'borderBottom', 'borderLeft', 'borderRight', @@ -314,7 +318,7 @@ export function applyTableBorderFormat( colIndex++ ) { const cell = tableModel.rows[sel.lastRow].cells[colIndex]; - applyBorderFormat(cell, borderFormat, [ + collectBorderFormat(cell, [ 'borderTop', 'borderLeft', 'borderRight', @@ -327,7 +331,7 @@ export function applyTableBorderFormat( rowIndex++ ) { const cell = tableModel.rows[rowIndex].cells[sel.firstColumn]; - applyBorderFormat(cell, borderFormat, [ + collectBorderFormat(cell, [ 'borderTop', 'borderBottom', isRtl ? 'borderLeft' : 'borderRight', @@ -340,7 +344,7 @@ export function applyTableBorderFormat( rowIndex++ ) { const cell = tableModel.rows[rowIndex].cells[sel.lastColumn]; - applyBorderFormat(cell, borderFormat, [ + collectBorderFormat(cell, [ 'borderTop', 'borderBottom', isRtl ? 'borderRight' : 'borderLeft', @@ -365,6 +369,20 @@ export function applyTableBorderFormat( } } + // Compare before changing any cells, and only consider borders targeted by + // this operation. Normalize CSS so DOM colors (rgb) also match hex input. + if ( + operation != 'noBorders' && + borderUpdates.length > 0 && + hasMatchingBorders(borderUpdates, borderFormat) + ) { + borderFormat = ''; + } + + for (const { cell, positions } of borderUpdates) { + applyBorderFormat(cell, borderFormat, positions); + } + //Format perimeter if necessary or possible modifyPerimeter(tableModel, sel, borderFormat, perimeter, isRtl); } @@ -386,6 +404,53 @@ export function applyTableBorderFormat( ); } +/** + * Check targeted borders without repeatedly parsing identical CSS values. + * @param borderUpdates The borders to compare + * @param borderFormat The requested border format + */ +function hasMatchingBorders(borderUpdates: BorderUpdate[], borderFormat: string): boolean { + const matchingBorders = new Set(); + + for (const { cell, positions } of borderUpdates) { + for (const pos of positions) { + const value = cell.format[pos] || ''; + + // Exact matches need no CSS parsing. Equivalent values are parsed only once. + if (value == borderFormat || matchingBorders.has(value)) { + continue; + } + + if (!areSameBorders(value, borderFormat)) { + return false; + } + + matchingBorders.add(value); + } + } + + return true; +} + +/** + * Compare border components, parsing colors to account for equivalent hex and RGB values. + */ +function areSameBorders(border1: string, border2: string): boolean { + const values1 = extractBorderValues(border1); + const values2 = extractBorderValues(border2); + const color1 = values1.color || ''; + const color2 = values2.color || ''; + const rgb1 = parseColor(color1); + const rgb2 = parseColor(color2); + + return ( + values1.width == values2.width && + values1.style == values2.style && + (color1 == color2 || + (!!rgb1 && !!rgb2 && rgb1[0] == rgb2[0] && rgb1[1] == rgb2[1] && rgb1[2] == rgb2[2])) + ); +} + /** * @internal * Apply border format to a cell diff --git a/packages/roosterjs-content-model-api/test/publicApi/table/applyTableBorderFormatTest.ts b/packages/roosterjs-content-model-api/test/publicApi/table/applyTableBorderFormatTest.ts index 20b9aba56038..a9b20ad68db3 100644 --- a/packages/roosterjs-content-model-api/test/publicApi/table/applyTableBorderFormatTest.ts +++ b/packages/roosterjs-content-model-api/test/publicApi/table/applyTableBorderFormatTest.ts @@ -44,6 +44,7 @@ describe('applyTableBorderFormat', () => { spyOn(normalizeTable, 'normalizeTable'); editor = ({} as any) as IEditor; + editor.getDocument = () => document; }); function runTest( @@ -78,6 +79,180 @@ describe('applyTableBorderFormat', () => { blocks: [expectedTable], }); } + describe('toggle borders', () => { + const operations: BorderOperations[] = [ + 'allBorders', + 'outsideBorders', + 'insideBorders', + 'topBorders', + 'bottomBorders', + 'leftBorders', + 'rightBorders', + ]; + const positions = ['borderTop', 'borderBottom', 'borderLeft', 'borderRight'] as const; + const originalBorder = '1px solid red'; + + function applyToTable( + table: ContentModelTable, + operation: BorderOperations, + border: Border = testBorder + ) { + const model = createContentModelDocument(); + model.blocks.push(table); + editor.formatContentModel = jasmine + .createSpy('formatContentModel') + .and.callFake((callback: ContentModelFormatter) => + callback(model, { newEntities: [], deletedEntities: [], newImages: [] }) + ); + applyTableBorderFormat(editor, border, operation); + } + + function copyTable(table: ContentModelTable): ContentModelTable { + return JSON.parse(JSON.stringify(table)); + } + + operations.forEach(operation => { + [false, true].forEach(isRtl => { + // Single cell, row, column, and grids with and without inner cells. + [ + [3, 3], + [3, 5], + [5, 3], + [4, 4], + [4, 5], + [5, 5], + ].forEach(([rows, columns]) => { + it(`${operation}, ${rows - 2}x${ + columns - 2 + }, RTL=${isRtl}: toggle off and on`, () => { + const table = createTestTable(rows, columns, { + borderTop: originalBorder, + borderBottom: originalBorder, + borderLeft: originalBorder, + borderRight: originalBorder, + }); + table.format.direction = isRtl ? 'rtl' : 'ltr'; + applyToTable(table, operation); + + const appliedTable = copyTable(table); + const clearedTable = copyTable(table); + + clearedTable.rows.forEach(row => + row.cells.forEach(cell => + positions.forEach(pos => { + if (cell.format[pos] == testBorderString) { + cell.format[pos] = ''; + } + }) + ) + ); + + // Untargeted borders and metadata must stay unchanged, while shared + // borders on cells outside the selection must also be cleared. + runTest(table, clearedTable, testBorder, operation); + runTest(table, appliedTable, testBorder, operation); + }); + }); + }); + + it(`${operation}: apply to the whole selection when one targeted border differs`, () => { + const table = createTestTable(5, 5); + applyToTable(table, operation); + const expectedTable = copyTable(table); + const cell = + table.rows[operation == 'bottomBorders' ? 3 : 1].cells[ + operation == 'rightBorders' ? 3 : 1 + ]; + const position = positions.filter(pos => cell.format[pos] == testBorderString)[0]; + + expect(position).toBeDefined(); + cell.format[position] = originalBorder; + runTest(table, expectedTable, testBorder, operation); + }); + }); + + it('ignores adjacent unselected cells when deciding whether to remove borders', () => { + const table = createTestTable(3, 3, { borderTop: testBorderString }); + const expectedTable = copyTable(table); + expectedTable.rows[1].cells[1].format.borderTop = ''; + expectedTable.rows[0].cells[1].format.borderBottom = ''; + expectedTable.rows[1].cells[1].dataset.editingInfo = '{"borderOverride":true}'; + expectedTable.rows[0].cells[1].dataset.editingInfo = '{"borderOverride":true}'; + + runTest(table, expectedTable, testBorder, 'topBorders'); + }); + + it('matches borders with equivalent hex and RGB colors', () => { + const table = createTestTable(3, 3); + applyToTable(table, 'outsideBorders'); + const expectedTable = copyTable(table); + + table.rows.forEach((row, rowIndex) => + row.cells.forEach((cell, colIndex) => + positions.forEach(pos => { + if (cell.format[pos] == testBorderString) { + cell.format[pos] = '3px double rgb(170, 187, 204)'; + expectedTable.rows[rowIndex].cells[colIndex].format[pos] = ''; + } + }) + ) + ); + + runTest(table, expectedTable, testBorder, 'outsideBorders'); + }); + + it('does not access the document when all targeted borders match exactly', () => { + const table = createTestTable(5, 5); + applyToTable(table, 'allBorders'); + const getDocument = spyOn(editor, 'getDocument').and.callThrough(); + + applyToTable(table, 'allBorders'); + + expect(getDocument).not.toHaveBeenCalled(); + expect(table.rows[1].cells[1].format.borderTop).toBe(''); + }); + + it('matches repeated equivalent borders', () => { + const rgbBorder = '3px double rgb(170, 187, 204)'; + const table = createTestTable(5, 5, { + borderTop: rgbBorder, + borderBottom: rgbBorder, + borderLeft: rgbBorder, + borderRight: rgbBorder, + }); + + applyToTable(table, 'allBorders'); + + expect(table.rows[1].cells[1].format.borderTop).toBe(''); + expect(table.rows[3].cells[3].format.borderBottom).toBe(''); + }); + + it('stops comparing at the first mismatch', () => { + const table = createTestTable(5, 5, { borderTop: originalBorder }); + + applyToTable(table, 'allBorders'); + + expect(table.rows[3].cells[3].format.borderBottom).toBe(testBorderString); + }); + + it('uses table border defaults when toggling', () => { + const table = createTestTable(3, 3); + table.format.borderTop = testBorderString; + applyToTable(table, 'topBorders', {}); + const expectedTable = copyTable(table); + expectedTable.rows[1].cells[1].format.borderTop = ''; + expectedTable.rows[0].cells[1].format.borderBottom = ''; + + runTest(table, expectedTable, {}, 'topBorders'); + }); + + it('noBorders keeps borders removed when applied repeatedly', () => { + const table = createTestTable(4, 4); + applyToTable(table, 'noBorders'); + runTest(table, copyTable(table), testBorder, 'noBorders'); + }); + }); + it('All Borders', () => { runTest( createTestTable(4, 4),