Fix Table first column (#2652)

* create Table Options button for demo

* fix setFirstColumnFormat

* fix dependency

* restore background color check for first column

* remove logs, move TableOptionsMenuItemStringKey, isHeader

* remove import

demo/scripts/controlsV2/demoButtons/tableOptionsButton.ts

@@ -0,0 +1,53 @@
+import { formatTable, getFormatState } from 'roosterjs-content-model-api';
+import { TableMetadataFormat } from 'roosterjs-content-model-types';
+import type { RibbonButton } from '../roosterjsReact/ribbon';
+
+const TableEditOperationMap: Partial<Record<
+    TableOptionsMenuItemStringKey,
+    keyof TableMetadataFormat
+>> = {
+    menuNameTableSetHeaderRow: 'hasHeaderRow',
+    menuNameTableSetFirstColumn: 'hasFirstColumn',
+    menuNameTableSetBandedColumns: 'hasBandedColumns',
+    menuNameTableSetBandedRows: 'hasBandedRows',
+};
+
+/**
+ * Key of localized strings of Table Options menu items
+ */
+type TableOptionsMenuItemStringKey =
+    | 'menuNameTableSetHeaderRow'
+    | 'menuNameTableSetFirstColumn'
+    | 'menuNameTableSetBandedColumns'
+    | 'menuNameTableSetBandedRows';
+
+export const tableOptionsButton: RibbonButton<
+    'ribbonButtonTableOptions' | TableOptionsMenuItemStringKey
+> = {
+    key: 'ribbonButtonTableOptions',
+    iconName: '',
+    unlocalizedText: 'Options',
+    isDisabled: formatState => !formatState.isInTable,
+    dropDownMenu: {
+        items: {
+            menuNameTableSetHeaderRow: 'Header Row',
+            menuNameTableSetFirstColumn: 'First Column',
+            menuNameTableSetBandedColumns: 'Banded Columns',
+            menuNameTableSetBandedRows: 'Banded Rows',
+        },
+    },
+    onClick: (editor, key) => {
+        if (key != 'ribbonButtonTableOptions') {
+            const format = getFormatState(editor);
+            const tableFormatProperty = TableEditOperationMap[key];
+            formatTable(
+                editor,
+                { [tableFormatProperty]: !format.tableFormat[tableFormatProperty] },
+                true /*keepCellShade*/
+            );
+        }
+    },
+    commandBarProperties: {
+        iconOnly: false,
+    },
+};

demo/scripts/controlsV2/tabs/ribbonButtons.ts

@@ -37,7 +37,6 @@ import { setBulletedListStyleButton } from '../demoButtons/setBulletedListStyleB
 import { setHeadingLevelButton } from '../roosterjsReact/ribbon/buttons/setHeadingLevelButton';
 import { setNumberedListStyleButton } from '../demoButtons/setNumberedListStyleButton';
 import { setTableCellShadeButton } from '../demoButtons/setTableCellShadeButton';
-import { setTableHeaderButton } from '../demoButtons/setTableHeaderButton';
 import { spaceAfterButton, spaceBeforeButton } from '../demoButtons/spaceBeforeAfterButtons';
 import { spacingButton } from '../demoButtons/spacingButton';
 import { strikethroughButton } from '../roosterjsReact/ribbon/buttons/strikethroughButton';
@@ -47,6 +46,7 @@ import { tableBorderApplyButton } from '../demoButtons/tableBorderApplyButton';
 import { tableBorderColorButton } from '../demoButtons/tableBorderColorButton';
 import { tableBorderStyleButton } from '../demoButtons/tableBorderStyleButton';
 import { tableBorderWidthButton } from '../demoButtons/tableBorderWidthButton';
+import { tableOptionsButton } from '../demoButtons/tableOptionsButton';
 import { tabNames } from './getTabs';
 import { textColorButton } from '../roosterjsReact/ribbon/buttons/textColorButton';
 import { underlineButton } from '../roosterjsReact/ribbon/buttons/underlineButton';
@@ -79,7 +79,7 @@ const tableButtons: RibbonButton<any>[] = [
     insertTableButton,
     formatTableButton,
     setTableCellShadeButton,
-    setTableHeaderButton,
+    tableOptionsButton,
     tableInsertButton,
     tableDeleteButton,
     tableBorderApplyButton,
@@ -169,7 +169,7 @@ const allButtons: RibbonButton<any>[] = [
     listStartNumberButton,
     formatTableButton,
     setTableCellShadeButton,
-    setTableHeaderButton,
+    tableOptionsButton,
     tableInsertButton,
     tableDeleteButton,
     tableMergeButton,

packages/roosterjs-content-model-api/lib/publicApi/table/applyTableBorderFormat.ts

@@ -2,7 +2,10 @@ import {
     extractBorderValues,
     getFirstSelectedTable,
     getSelectedCells,
+    getTableMetadata,
+    hasMetadata,
     parseValueWithUnit,
+    setFirstColumnFormatBorders,
     updateTableCellMetadata,
 } from 'roosterjs-content-model-dom';
 import type {
@@ -366,6 +369,12 @@ export function applyTableBorderFormat(
                     modifyPerimeter(tableModel, sel, borderFormat, perimeter, isRtl);
                 }
 
+                const tableMeta = hasMetadata(tableModel) ? getTableMetadata(tableModel) : {};
+                if (tableMeta) {
+                    // Enforce first column format if necessary
+                    setFirstColumnFormatBorders(tableModel.rows, tableMeta);
+                }
+
                 return true;
             } else {
                 return false;

packages/roosterjs-content-model-api/test/publicApi/table/applyTableBorderFormatTest.ts

@@ -26,7 +26,7 @@ describe('applyTableBorderFormat', () => {
         format?: ContentModelTableCell['format']
     ) {
         // Create a table with all cells selected except the first and last row and column
-        const table = createTable(rows);
+        const table: ContentModelTable = createTable(rows);
         for (let i = 0; i < rows; i++) {
             const row = table.rows[i];
             for (let j = 0; j < columns; j++) {

packages/roosterjs-content-model-dom/lib/index.ts

@@ -124,7 +124,7 @@ export { mergeModel } from './modelApi/editing/mergeModel';
 export { deleteSelection } from './modelApi/editing/deleteSelection';
 export { deleteSegment } from './modelApi/editing/deleteSegment';
 export { deleteBlock } from './modelApi/editing/deleteBlock';
-export { applyTableFormat } from './modelApi/editing/applyTableFormat';
+export { applyTableFormat, setFirstColumnFormatBorders } from './modelApi/editing/applyTableFormat';
 export { normalizeTable, MIN_ALLOWED_TABLE_CELL_WIDTH } from './modelApi/editing/normalizeTable';
 export { setTableCellBackgroundColor } from './modelApi/editing/setTableCellBackgroundColor';
 export { retrieveModelFormatState } from './modelApi/editing/retrieveModelFormatState';

packages/roosterjs-content-model-dom/lib/modelApi/editing/applyTableFormat.ts

@@ -58,7 +58,7 @@ export function applyTableFormat(
 
         clearCache(rows);
         formatCells(rows, effectiveMetadata, metaOverrides);
-        setFirstColumnFormat(rows, effectiveMetadata, metaOverrides);
+        setFirstColumnFormatBorders(rows, effectiveMetadata);
         setHeaderRowFormat(rows, effectiveMetadata, metaOverrides);
         return effectiveMetadata;
     });
@@ -176,7 +176,7 @@ function formatCells(
     format: TableMetadataFormat,
     metaOverrides: MetaOverrides
 ) {
-    const { hasBandedRows, hasBandedColumns, bgColorOdd, bgColorEven } = format;
+    const { hasBandedRows, hasBandedColumns, bgColorOdd, bgColorEven, hasFirstColumn } = format;
 
     rows.forEach((row, rowIndex) => {
         row.cells.forEach((cell, colIndex) => {
@@ -212,14 +212,18 @@ function formatCells(
 
             // Format Background Color
             if (!metaOverrides.bgColorOverrides[rowIndex][colIndex]) {
-                const color =
-                    hasBandedRows || hasBandedColumns
-                        ? (hasBandedColumns && colIndex % 2 != 0) ||
-                          (hasBandedRows && rowIndex % 2 != 0)
-                            ? bgColorOdd
-                            : bgColorEven
-                        : bgColorEven; /* bgColorEven is the default color */
-
+                let color: string | null | undefined;
+                if (hasFirstColumn && colIndex == 0) {
+                    color = null;
+                } else {
+                    color =
+                        hasBandedRows || hasBandedColumns
+                            ? (hasBandedColumns && colIndex % 2 != 0) ||
+                              (hasBandedRows && rowIndex % 2 != 0)
+                                ? bgColorOdd
+                                : bgColorEven
+                            : bgColorEven; /* bgColorEven is the default color */
+                }
                 setTableCellBackgroundColor(
                     cell,
                     color,
@@ -232,35 +236,46 @@ function formatCells(
             if (format.verticalAlign && !metaOverrides.vAlignOverrides[rowIndex][colIndex]) {
                 cell.format.verticalAlign = format.verticalAlign;
             }
+
+            // Format Header
+            cell.isHeader = false;
         });
     });
 }
 
-function setFirstColumnFormat(
+/**
+ * Set the first column format borders for the table
+ * @param rows The rows of the table
+ * @param format The table metadata format
+ */
+export function setFirstColumnFormatBorders(
     rows: ContentModelTableRow[],
-    format: Partial<TableMetadataFormat>,
-    metaOverrides: MetaOverrides
+    format: Partial<TableMetadataFormat>
 ) {
+    // Exit early hasFirstColumn is not set
+    if (!format.hasFirstColumn) {
+        return;
+    }
+
     rows.forEach((row, rowIndex) => {
         row.cells.forEach((cell, cellIndex) => {
-            if (format.hasFirstColumn && cellIndex === 0) {
+            if (cellIndex == 0) {
                 cell.isHeader = true;
 
-                if (rowIndex !== 0 && !metaOverrides.bgColorOverrides[rowIndex][cellIndex]) {
-                    setBorderColor(cell.format, 'borderTop');
-                    setTableCellBackgroundColor(
-                        cell,
-                        null /*color*/,
-                        false /*isColorOverride*/,
-                        true /*applyToSegments*/
-                    );
-                }
-
-                if (rowIndex !== rows.length - 1 && rowIndex !== 0) {
-                    setBorderColor(cell.format, 'borderBottom');
+                switch (rowIndex) {
+                    case 0:
+                        break;
+                    case 1:
+                        setBorderColor(cell.format, 'borderBottom');
+                        break;
+                    case rows.length - 1:
+                        setBorderColor(cell.format, 'borderTop');
+                        break;
+                    default:
+                        setBorderColor(cell.format, 'borderTop');
+                        setBorderColor(cell.format, 'borderBottom');
+                        break;
                 }
-            } else {
-                cell.isHeader = false;
             }
         });
     });
@@ -271,12 +286,17 @@ function setHeaderRowFormat(
     format: TableMetadataFormat,
     metaOverrides: MetaOverrides
 ) {
+    // Exit early if hasHeaderRow is not set
+    if (!format.hasHeaderRow) {
+        return;
+    }
+
     const rowIndex = 0;
 
     rows[rowIndex]?.cells.forEach((cell, cellIndex) => {
-        cell.isHeader = format.hasHeaderRow;
+        cell.isHeader = true;
 
-        if (format.hasHeaderRow && format.headerRowColor) {
+        if (format.headerRowColor) {
             if (!metaOverrides.bgColorOverrides[rowIndex][cellIndex]) {
                 setTableCellBackgroundColor(
                     cell,
@@ -293,6 +313,11 @@ function setHeaderRowFormat(
     });
 }
 
+/**
+ * @param format The cell format to set the border color
+ * @param key The border key to set the color
+ * @param value The color to set. If not given, it removes the color and sets the style to transparent
+ */
 function setBorderColor(format: BorderFormat, key: keyof BorderFormat, value?: string) {
     const border = extractBorderValues(format[key]);
     border.color = value || '';

packages/roosterjs-content-model-types/lib/contentModel/format/metadata/TableMetadataFormat.ts

@@ -56,6 +56,7 @@ export type TableMetadataFormat = {
      * Table Borders Type. Use value of constant TableBorderFormat as value
      */
     tableBorderFormat?: number;
+
     /**
      * Vertical alignment for each row
      */