Merge pull request #25548 from overleaf/mj-add-booktabs

[web] Add support for booktabs table style

GitOrigin-RevId: e3f7e1a867474a86e4b5f8c701d845d55592bb68

services/web/frontend/extracted-translations.json

@@ -183,6 +183,7 @@
   "blocked_filename": "",
   "blog": "",
   "bold": "",
+  "booktabs": "",
   "browser": "",
   "bullet_list": "",
   "buy_licenses": "",
@@ -1035,7 +1036,6 @@
   "more_editor_toolbar_item": "",
   "more_info": "",
   "more_options": "",
-  "more_options_for_border_settings_coming_soon": "",
   "my_library": "",
   "n_items": "",
   "n_items_plural": "",

services/web/frontend/js/features/source-editor/components/table-generator/tabular.tsx

@@ -139,6 +139,33 @@ export class TableData {
     if (this.rows.length === 0 || this.columns.length === 0) {
       return null
     }
+    const hasTopRule = this.rows[0].borderTop === 1
+    const hasMidRule =
+      this.rows[1]?.borderTop === 1 &&
+      (this.rows.length === 2 || this.rows[1]?.borderBottom === 0)
+    const hasBottomRule =
+      this.rows[this.rows.length - 1].borderBottom === 1 &&
+      (this.rows.length === 2 ||
+        this.rows[this.rows.length - 1].borderTop === 0)
+
+    if (hasTopRule && hasMidRule && hasBottomRule) {
+      let isBooktabs = true
+      // Check for no other borders
+      for (const row of this.rows.slice(2, -1)) {
+        if (row.borderTop > 0 || row.borderBottom > 0) {
+          isBooktabs = false
+        }
+      }
+      for (const column of this.columns) {
+        if (column.borderLeft > 0 || column.borderRight > 0) {
+          isBooktabs = false
+        }
+      }
+      if (isBooktabs) {
+        return BorderTheme.BOOKTABS
+      }
+    }
+
     const lastRow = this.rows[this.rows.length - 1]
     const hasBottomBorder = lastRow.borderBottom > 0
     const firstColumn = this.columns[0]
@@ -180,8 +207,10 @@ export class TableData {
 
     if (hasAllRowBorders && hasAllColumnBorders) {
       return BorderTheme.FULLY_BORDERED
-    } else {
+    } else if (!hasAllRowBorders && !hasAllColumnBorders) {
       return BorderTheme.NO_BORDERS
+    } else {
+      return null
     }
   }
 }

services/web/frontend/js/features/source-editor/components/table-generator/toolbar/commands.ts

@@ -20,8 +20,224 @@ import { WidthSelection } from './column-width-modal/column-width'
 export enum BorderTheme {
   NO_BORDERS = 0,
   FULLY_BORDERED = 1,
+  BOOKTABS = 2,
 }
 /* eslint-enable no-unused-vars */
+
+type ThemeGenerator = {
+  column: (
+    number: number,
+    numColumns: number
+  ) => { left: boolean; right: boolean }
+  row: (number: number, numRows: number) => string | false
+  lastRow?: () => string | false
+  multicolumn: () => { left: boolean; right: boolean }
+}
+
+const themeGenerators: Record<BorderTheme, ThemeGenerator> = {
+  [BorderTheme.NO_BORDERS]: {
+    column: () => ({ left: false, right: false }),
+    row: () => false,
+    multicolumn: () => ({ left: false, right: false }),
+  },
+  [BorderTheme.FULLY_BORDERED]: {
+    column: (number: number, numColumns: number) => ({
+      left: true,
+      right: number === numColumns - 1,
+    }),
+    row: (number: number, numRows: number) => '\\hline',
+    multicolumn: () => ({ left: true, right: true }),
+    lastRow: () => '\\hline',
+  },
+  [BorderTheme.BOOKTABS]: {
+    column: (number: number, numColumns: number) => ({
+      left: false,
+      right: false,
+    }),
+    row: (number: number, numRows: number) => {
+      if (number === 0) {
+        return '\\toprule'
+      }
+      if (number === 1) {
+        return '\\midrule'
+      }
+      return false
+    },
+    lastRow: () => '\\bottomrule',
+    multicolumn: () => ({ left: false, right: false }),
+  },
+}
+
+function applyBorderTheme(
+  generator: ThemeGenerator,
+  view: EditorView,
+  table: TableData,
+  positions: Positions,
+  rowSeparators: RowSeparator[]
+): ChangeSpec[] {
+  const changes: ChangeSpec[] = []
+
+  // Update specification
+  const spec = view.state.sliceDoc(
+    positions.columnDeclarations.from,
+    positions.columnDeclarations.to
+  )
+  const columnSpecification = parseColumnSpecifications(spec)
+  columnSpecification.forEach((column, index) => {
+    const { left, right } = generator.column(index, columnSpecification.length)
+    column.borderLeft = left ? 1 : 0
+    column.borderRight = right ? 1 : 0
+  })
+  const newSpec = generateColumnSpecification(columnSpecification)
+  if (newSpec !== spec) {
+    changes.push({
+      from: positions.columnDeclarations.from,
+      to: positions.columnDeclarations.to,
+      insert: newSpec,
+    })
+  }
+
+  for (let i = 0; i < positions.rowPositions.length; i++) {
+    const row = positions.rowPositions[i]
+    const topBorder = generator.row(i, positions.rowPositions.length)
+    if (topBorder) {
+      let borderPresent = false
+      for (const hline of row.hlines) {
+        if (hline.from > rowSeparators[i]?.to) {
+          continue
+        }
+        if (borderPresent) {
+          // Remove extra border
+          changes.push({
+            from: hline.from,
+            to: hline.to,
+            insert: '',
+          })
+        } else {
+          const type = view.state.sliceDoc(hline.from, hline.to)
+          if (type.trim() !== topBorder) {
+            // Replace with the correct border
+            changes.push({
+              from: hline.from,
+              to: hline.to,
+              insert: topBorder,
+            })
+          }
+          borderPresent = true
+        }
+      }
+      if (!borderPresent) {
+        // Add the border
+        changes.push({
+          from: row.from,
+          to: row.from,
+          insert: topBorder,
+        })
+      }
+    } else {
+      for (const hline of row.hlines) {
+        if (hline.from > rowSeparators[i]?.to) {
+          continue
+        }
+        changes.push({
+          from: hline.from,
+          to: hline.to,
+          insert: '',
+        })
+      }
+    }
+  }
+
+  const lastRow = positions.rowPositions[positions.rowPositions.length - 1]
+  const lastRowBorder = generator.lastRow?.()
+  const hasLastRowSeparator =
+    positions.rowPositions.length === rowSeparators.length
+  if (hasLastRowSeparator) {
+    if (lastRowBorder) {
+      let borderPresent = false
+      for (const hline of lastRow.hlines) {
+        if (hline.from < rowSeparators[positions.rowPositions.length - 1].to) {
+          continue
+        }
+        if (borderPresent) {
+          // Remove extra border
+          changes.push({
+            from: hline.from,
+            to: hline.to,
+            insert: '',
+          })
+        } else {
+          const type = view.state.sliceDoc(hline.from, hline.to)
+          if (type.trim() !== lastRowBorder) {
+            // Replace with the correct border
+            changes.push({
+              from: hline.from,
+              to: hline.to,
+              insert: lastRowBorder,
+            })
+          }
+          borderPresent = true
+        }
+      }
+      if (!borderPresent) {
+        const rowSeparator = rowSeparators[positions.rowPositions.length - 1]
+
+        // Add the border
+        changes.push({
+          from: rowSeparator.to,
+          to: rowSeparator.to,
+          insert: ` ${lastRowBorder}`,
+        })
+      }
+    } else {
+      for (const hline of lastRow.hlines) {
+        if (hline.from < rowSeparators[positions.rowPositions.length - 1].to) {
+          continue
+        }
+        changes.push({
+          from: hline.from,
+          to: hline.to,
+          insert: '',
+        })
+      }
+    }
+  } else if (lastRowBorder) {
+    changes.push({
+      from: lastRow.to,
+      to: lastRow.to,
+      insert: `\\\\ ${lastRowBorder}`,
+    })
+  }
+
+  // Update multicolumn
+  for (const row of table.rows) {
+    for (const cell of row.cells) {
+      if (cell.multiColumn) {
+        const { left, right } = generator.multicolumn()
+        const spec = view.state.sliceDoc(
+          cell.multiColumn.columns.from,
+          cell.multiColumn.columns.to
+        )
+        const columnSpecification = parseColumnSpecifications(spec)
+        columnSpecification.forEach(column => {
+          column.borderLeft = left ? 1 : 0
+          column.borderRight = right ? 1 : 0
+        })
+        const newSpec = generateColumnSpecification(columnSpecification)
+        if (newSpec !== spec) {
+          changes.push({
+            from: cell.multiColumn.columns.from,
+            to: cell.multiColumn.columns.to,
+            insert: newSpec,
+          })
+        }
+      }
+    }
+  }
+
+  return changes
+}
+
 export const setBorders = (
   view: EditorView,
   theme: BorderTheme,
@@ -29,118 +245,18 @@ export const setBorders = (
   rowSeparators: RowSeparator[],
   table: TableData
 ) => {
-  const specification = view.state.sliceDoc(
-    positions.columnDeclarations.from,
-    positions.columnDeclarations.to
+  const generator = themeGenerators[theme]
+  const changes = applyBorderTheme(
+    generator,
+    view,
+    table,
+    positions,
+    rowSeparators
   )
-  if (theme === BorderTheme.NO_BORDERS) {
-    const removeColumnBorders = view.state.changes({
-      from: positions.columnDeclarations.from,
-      to: positions.columnDeclarations.to,
-      insert: specification.replace(/\|/g, ''),
-    })
-    const removeHlines: ChangeSpec[] = []
-    for (const row of positions.rowPositions) {
-      for (const hline of row.hlines) {
-        removeHlines.push({
-          from: hline.from,
-          to: hline.to,
-          insert: '',
-        })
-      }
-    }
-    const removeMulticolumnBorders: ChangeSpec[] = []
-    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
-          )
-          removeMulticolumnBorders.push({
-            from: cell.multiColumn.columns.from,
-            to: cell.multiColumn.columns.to,
-            insert: specification.replace(/\|/g, ''),
-          })
-        }
-      }
-    }
-    view.dispatch({
-      changes: [
-        removeColumnBorders,
-        ...removeHlines,
-        ...removeMulticolumnBorders,
-      ],
-    })
-  } else if (theme === BorderTheme.FULLY_BORDERED) {
-    const newSpec = generateColumnSpecification(
-      addColumnBordersToSpecification(table.columns)
-    )
 
-    const insertColumns = view.state.changes({
-      from: positions.columnDeclarations.from,
-      to: positions.columnDeclarations.to,
-      insert: newSpec,
-    })
-
-    const insertHlines: ChangeSpec[] = []
-    for (const row of positions.rowPositions) {
-      if (row.hlines.length === 0) {
-        insertHlines.push(
-          view.state.changes({
-            from: row.from,
-            to: row.from,
-            insert: ' \\hline ',
-          })
-        )
-      }
-    }
-    const lastRow = positions.rowPositions[positions.rowPositions.length - 1]
-    if (lastRow.hlines.length < 2) {
-      let toInsert = ' \\hline'
-      if (rowSeparators.length < positions.rowPositions.length) {
-        // We need a trailing \\
-        toInsert = ` \\\\${toInsert}`
-      }
-      insertHlines.push(
-        view.state.changes({
-          from: lastRow.to,
-          to: lastRow.to,
-          insert: toInsert,
-        })
-      )
-    }
-    const addMulticolumnBorders: ChangeSpec[] = []
-    for (const row of table.rows) {
-      for (const cell of row.cells) {
-        if (cell.multiColumn) {
-          addMulticolumnBorders.push({
-            from: cell.multiColumn.columns.from,
-            to: cell.multiColumn.columns.to,
-            insert: generateColumnSpecification(
-              addColumnBordersToSpecification(
-                cell.multiColumn.columns.specification
-              )
-            ),
-          })
-        }
-      }
-    }
-
-    view.dispatch({
-      changes: [insertColumns, ...insertHlines, ...addMulticolumnBorders],
-    })
-  }
-}
-
-const addColumnBordersToSpecification = (specification: ColumnDefinition[]) => {
-  const newSpec = specification.map(column => ({
-    ...column,
-    borderLeft: 1,
-    borderRight: 0,
-  }))
-  newSpec[newSpec.length - 1].borderRight = 1
-  return newSpec
+  view.dispatch({
+    changes,
+  })
 }
 
 export const setAlignment = (

services/web/frontend/js/features/source-editor/components/table-generator/toolbar/toolbar.tsx

@@ -3,7 +3,6 @@ import { useSelectionContext } from '../contexts/selection-context'
 import { ToolbarButton } from './toolbar-button'
 import { ToolbarButtonMenu } from './toolbar-button-menu'
 import { ToolbarDropdown, ToolbarDropdownItem } from './toolbar-dropdown'
-import MaterialIcon from '../../../../../shared/components/material-icon'
 import {
   BorderTheme,
   insertColumn,
@@ -47,6 +46,8 @@ export const Toolbar = memo(function Toolbar() {
         return t('all_borders')
       case BorderTheme.NO_BORDERS:
         return t('no_borders')
+      case BorderTheme.BOOKTABS:
+        return t('booktabs')
       default:
         return t('custom_borders')
     }
@@ -203,12 +204,22 @@ export const Toolbar = memo(function Toolbar() {
           >
             {t('no_borders')}
           </ToolbarDropdownItem>
-          <div className="table-generator-border-options-coming-soon">
-            <div className="info-icon">
-              <MaterialIcon type="info" />
-            </div>
-            {t('more_options_for_border_settings_coming_soon')}
-          </div>
+          <ToolbarDropdownItem
+            id="table-generator-borders-booktabs"
+            command={() => {
+              setBorders(
+                view,
+                BorderTheme.BOOKTABS,
+                positions,
+                rowSeparators,
+                table
+              )
+            }}
+            active={table.getBorderTheme() === BorderTheme.BOOKTABS}
+            icon="border_top"
+          >
+            {t('booktabs')}
+          </ToolbarDropdownItem>
         </ToolbarDropdown>
       </div>
       <div className="table-generator-button-group">

services/web/locales/en.json

@@ -237,6 +237,7 @@
   "blocked_filename": "This file name is blocked.",
   "blog": "Blog",
   "bold": "Bold",
+  "booktabs": "Booktabs",
   "brl_discount_offer_plans_page_banner": "__flag__ <b>Great news!</b> We’ve applied a 50% discount to premium plans on this page for our users in Brazil. Check out the new lower prices.",
   "browser": "Browser",
   "built_in": "Built-In",
@@ -1353,7 +1354,6 @@
   "more_editor_toolbar_item": "More editor toolbar items",
   "more_info": "More Info",
   "more_options": "More options",
-  "more_options_for_border_settings_coming_soon": "More options for border settings coming soon.",
   "more_project_collaborators": "<0>More</0> project <0>collaborators</0>",
   "more_than_one_kind_of_snippet_was_requested": "The link to open this content on Overleaf included some invalid parameters. If this keeps happening for links on a particular site, please report this to them.",
   "most_popular_uppercase": "Most popular",

services/web/test/frontend/features/source-editor/components/codemirror-editor-table-generator.spec.tsx

@@ -217,6 +217,41 @@ cell 3 & cell 4 \\\\
       )
     })
 
+    it('Correctly sets borders for booktabs', function () {
+      // Add a blank line above the table to allow room for the table toolbar
+      mountEditor(`
+
+\\begin{tabular}{c|c}
+    cell 1 & cell 2 \\\\
+    cell 3 & cell 4 \\\\
+    cell 5 & cell 6 \\\\
+\\end{tabular}
+`)
+      checkBordersWithNoMultiColumn(
+        [false, false, false, false],
+        [false, true, false]
+      )
+      cy.get('.table-generator-floating-toolbar').should('not.exist')
+      cy.get('.table-generator-cell').first().click()
+      cy.get('.table-generator-floating-toolbar').as('toolbar').should('exist')
+      cy.get('@toolbar').findByText('Custom borders').click({ force: true })
+      cy.get('.table-generator').findByText('Booktabs').click()
+      // The element is partially covered, but we can still click it
+      cy.get('.cm-line').first().click({ force: true })
+      // Table should be unchanged
+      checkTable([
+        ['cell 1', 'cell 2'],
+        ['cell 3', 'cell 4'],
+        ['cell 5', 'cell 6'],
+      ])
+      checkBordersWithNoMultiColumn(
+        [true, true, false, true],
+        [false, false, false]
+      )
+      cy.get('.table-generator-cell').first().click()
+      cy.get('@toolbar').findByText('Booktabs').should('exist')
+    })
+
     it('Changes the column alignment with dropdown buttons', function () {
       // Add a blank line above the table to allow room for the table toolbar
       mountEditor(`