Merge pull request #14548 from overleaf/mj-early-exit-table-rendering

[visual] Show code when table generator fails rendering GitOrigin-RevId: 6c1908b0c68cc965e445736f0c320f322d23c988
2024-11-07 20:31:06 -05:00 · 2023-09-04 09:02:31 +01:00 · 2023-09-04 09:02:31 +01:00 · 8e6d6f8689
commit 8e6d6f8689
parent 601365bcc6
7 changed files with 125 additions and 56 deletions
--- a/services/web/frontend/js/features/source-editor/components/table-generator/contexts/table-context.tsx
+++ b/services/web/frontend/js/features/source-editor/components/table-generator/contexts/table-context.tsx
@ -3,9 +3,9 @@ import { Positions, TableData, TableRenderingError } from '../tabular'
 import {
  CellPosition,
  CellSeparator,
+  ParsedTableData,
  RowPosition,
  RowSeparator,
-  generateTable,
  parseTableEnvironment,
 } from '../utils'
 import { EditorView } from '@codemirror/view'
@ -34,34 +34,12 @@ const TableContext = createContext<
 >(undefined)

 export const TableProvider: FC<{
+  tableData: ParsedTableData
+  tableNode: SyntaxNode | null
  tabularNode: SyntaxNode
  view: EditorView
-  tableNode: SyntaxNode | null
-}> = ({ tabularNode, view, children, tableNode }) => {
+}> = ({ tableData, children, tableNode, tabularNode, view }) => {
  try {
-    const tableData = generateTable(tabularNode, view.state)
-
-    // TODO: Validate better that the table matches the column definition
-    for (const row of tableData.table.rows) {
-      const rowLength = row.cells.reduce(
-        (acc, cell) => acc + (cell.multiColumn?.columnSpan ?? 1),
-        0
-      )
-      for (const cell of row.cells) {
-        if (
-          cell.multiColumn?.columns.specification &&
-          cell.multiColumn.columns.specification.length !== 1
-        ) {
-          throw new Error(
-            'Multi-column cells must have exactly one column definition'
-          )
-        }
-      }
-      if (rowLength !== tableData.table.columns.length) {
-        throw new Error('Row length does not match column definition')
-      }
-    }
-
    const positions: Positions = {
      cells: tableData.cellPositions,
      columnDeclarations: tableData.specification,
--- 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
@ -1,6 +1,6 @@
 import { SyntaxNode } from '@lezer/common'
 import { FC, useEffect } from 'react'
-import { CellPosition, RowPosition } from './utils'
+import { CellPosition, ParsedTableData, RowPosition } from './utils'
 import { Toolbar } from './toolbar/toolbar'
 import { Table } from './table'
 import {
@ -182,7 +182,8 @@ export const Tabular: FC<{
  tabularNode: SyntaxNode
  view: EditorView
  tableNode: SyntaxNode | null
-}> = ({ tabularNode, view, tableNode }) => {
+  parsedTableData: ParsedTableData
+}> = ({ tabularNode, view, tableNode, parsedTableData }) => {
  return (
    <ErrorBoundary
      fallbackRender={() => (
@ -193,8 +194,9 @@ export const Tabular: FC<{
        <TabularProvider>
          <TableProvider
            tabularNode={tabularNode}
-            view={view}
+            tableData={parsedTableData}
            tableNode={tableNode}
+            view={view}
          >
            <SelectionContextProvider>
              <EditingContextProvider>
--- 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
@ -315,17 +315,19 @@ function parseTabularBody(
  return body
 }

-export function generateTable(
-  node: SyntaxNode,
-  state: EditorState
-): {
+export type ParsedTableData = {
  table: TableData
  cellPositions: CellPosition[][]
  specification: { from: number; to: number }
  rowPositions: RowPosition[]
  rowSeparators: RowSeparator[]
  cellSeparators: CellSeparator[][]
-} {
+}
+
+export function generateTable(
+  node: SyntaxNode,
+  state: EditorState
+): ParsedTableData {
  const specification = node
    .getChild('BeginEnv')
    ?.getChild('TextArgument')
--- a/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts
+++ b/services/web/frontend/js/features/source-editor/extensions/visual/atomic-decorations.ts
@ -65,6 +65,7 @@ import { IndicatorWidget } from './visual-widgets/indicator'
 import { TabularWidget } from './visual-widgets/tabular'
 import { nextSnippetField, pickedCompletion } from '@codemirror/autocomplete'
 import { skipPreambleWithCursor } from './skip-preamble-cursor'
+import { TableRenderingErrorWidget } from './visual-widgets/table-rendering-error'

 type Options = {
  fileTreeManager: {
@ -323,20 +324,33 @@ export const atomicDecorations = (options: Options) => {
                nodeRef.node,
                'TableEnvironment'
              )
-              decorations.push(
-                Decoration.replace({
-                  widget: new TabularWidget(
-                    nodeRef.node,
-                    state.doc.sliceString(
-                      (tableNode ?? nodeRef).from,
-                      (tableNode ?? nodeRef).to
-                    ),
-                    tableNode
-                  ),
-                  block: true,
-                }).range(nodeRef.from, nodeRef.to)
+              const tabularWidget = new TabularWidget(
+                nodeRef.node,
+                state.doc.sliceString(
+                  (tableNode ?? nodeRef).from,
+                  (tableNode ?? nodeRef).to
+                ),
+                tableNode,
+                state
              )
-              return false
+
+              if (tabularWidget.isValid()) {
+                decorations.push(
+                  Decoration.replace({
+                    widget: tabularWidget,
+                    block: true,
+                  }).range(nodeRef.from, nodeRef.to)
+                )
+                return false
+              } else {
+                // Show error message
+                decorations.push(
+                  Decoration.widget({
+                    widget: new TableRenderingErrorWidget(tableNode),
+                    block: true,
+                  }).range(nodeRef.from, nodeRef.from)
+                )
+              }
            }
          }
        } else if (nodeRef.type.is('BeginEnv')) {
--- a/services/web/frontend/js/features/source-editor/extensions/visual/table-generator.ts
+++ b/services/web/frontend/js/features/source-editor/extensions/visual/table-generator.ts
@ -1,7 +1,7 @@
 import { EditorView } from '@codemirror/view'

 export const tableGeneratorTheme = EditorView.baseTheme({
-  '&dark .table-generator': {
+  '&dark .table-generator-container': {
    '--table-generator-active-border-color': '#ccc',
    '--table-generator-coming-soon-background-color': '#41464f',
    '--table-generator-coming-soon-color': '#fff',
@ -23,6 +23,7 @@ export const tableGeneratorTheme = EditorView.baseTheme({
      'rgba(125,125,125,0.3)',
    '--table-generator-toolbar-dropdown-disabled-color': '#999',
    '--table-generator-toolbar-shadow-color': '#1e253029',
+    '--table-generator-error-background': '#F1F4F9',
  },

  '&light .table-generator': {
@ -46,6 +47,7 @@ export const tableGeneratorTheme = EditorView.baseTheme({
    '--table-generator-toolbar-dropdown-disabled-background': '#f2f2f2',
    '--table-generator-toolbar-dropdown-disabled-color': 'var(--neutral-40)',
    '--table-generator-toolbar-shadow-color': '#1e253029',
+    '--table-generator-error-background': '#F1F4F9',
  },

  '.table-generator': {
@ -401,20 +403,24 @@ export const tableGeneratorTheme = EditorView.baseTheme({
    },
  },

+  '.ol-cm-environment-table.table-generator-error-container, .ol-cm-environment-table.ol-cm-tabular':
+    {
+      background: 'rgba(125, 125, 125, 0.05)',
+    },
+
  '.table-generator-error': {
-    background:
-      'linear-gradient(0deg, #f9f1f1, #f9f1f1), linear-gradient(0deg, #f5beba, #f5beba)',
+    background: 'var(--table-generator-error-background)',
    display: 'flex',
    'justify-content': 'space-between',
    color: 'black',
-    border: '1px solid #f5beba',
+    border: '1px solid #C3D0E3',
    'font-family': 'Lato',
-    'margin-bottom': '0',
+    margin: '0 16px 0 16px',
    '& .table-generator-error-message': {
-      flex: '1 0 auto',
+      flex: '1 1 auto',
    },
    '& .table-generator-error-icon': {
-      color: '#b83a33',
+      color: '#3265B2',
      'margin-right': '12px',
    },
  },
--- a/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/table-rendering-error.ts
+++ b/services/web/frontend/js/features/source-editor/extensions/visual/visual-widgets/table-rendering-error.ts
@ -0,0 +1,34 @@
+import { WidgetType } from '@codemirror/view'
+import { SyntaxNode } from '@lezer/common'
+
+export class TableRenderingErrorWidget extends WidgetType {
+  private hasTableNode: boolean
+  constructor(tableNode: SyntaxNode | null | undefined) {
+    super()
+    this.hasTableNode = Boolean(tableNode)
+  }
+
+  toDOM(): HTMLElement {
+    const warning = document.createElement('div')
+    warning.classList.add('table-generator-error', 'alert')
+    warning.role = 'alert'
+    const icon = document.createElement('span')
+    icon.classList.add('table-generator-error-icon')
+    const iconType = document.createElement('i')
+    iconType.classList.add('fa', 'fa-info-circle')
+    icon.appendChild(iconType)
+    warning.appendChild(icon)
+    const message = document.createElement('span')
+    message.classList.add('table-generator-error-message')
+    message.textContent =
+      'We couldn’t render your table.\nThis could be because some features of this table are not supported in the table preview yet, or due to a LaTeX error in the table code.'
+    warning.appendChild(message)
+    const element = document.createElement('div')
+    element.classList.add('table-generator', 'table-generator-error-container')
+    element.appendChild(warning)
+    if (this.hasTableNode) {
+      element.classList.add('ol-cm-environment-table')
+    }
+    return element
+  }
+}
--- 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
@ -1,27 +1,59 @@
 import { EditorView, WidgetType } from '@codemirror/view'
+import { EditorState } from '@codemirror/state'
 import { SyntaxNode } from '@lezer/common'
 import * as ReactDOM from 'react-dom'
 import { Tabular } from '../../../components/table-generator/tabular'
+import {
+  ParsedTableData,
+  generateTable,
+} from '../../../components/table-generator/utils'

 export class TabularWidget extends WidgetType {
  private element: HTMLElement | undefined
+  private readonly parseResult: ParsedTableData

  constructor(
    private tabularNode: SyntaxNode,
    private content: string,
-    private tableNode: SyntaxNode | null
+    private tableNode: SyntaxNode | null,
+    state: EditorState
  ) {
    super()
+    this.parseResult = generateTable(tabularNode, state)
+  }
+
+  isValid() {
+    for (const row of this.parseResult.table.rows) {
+      const rowLength = row.cells.reduce(
+        (acc, cell) => acc + (cell.multiColumn?.columnSpan ?? 1),
+        0
+      )
+      for (const cell of row.cells) {
+        if (
+          cell.multiColumn?.columns.specification &&
+          cell.multiColumn.columns.specification.length !== 1
+        ) {
+          return false
+        }
+      }
+      if (rowLength !== this.parseResult.table.columns.length) {
+        return false
+      }
+    }
+    return true
  }

  toDOM(view: EditorView) {
    this.element = document.createElement('div')
    this.element.classList.add('ol-cm-tabular')
-    this.element.style.backgroundColor = 'rgba(125, 125, 125, 0.05)'
+    if (this.tableNode) {
+      this.element.classList.add('ol-cm-environment-table')
+    }
    ReactDOM.render(
      <Tabular
        view={view}
        tabularNode={this.tabularNode}
+        parsedTableData={this.parseResult}
        tableNode={this.tableNode}
      />,
      this.element
@ -44,6 +76,7 @@ export class TabularWidget extends WidgetType {
      <Tabular
        view={view}
        tabularNode={this.tabularNode}
+        parsedTableData={this.parseResult}
        tableNode={this.tableNode}
      />,
      this.element