Quick Editing in Table Editor Widget (#12129)

Fixes #10865 The proper "tabbing" cells is implemented in AgGrid, but wasn't visible due to a bug. This PR adds only header edit handling. "Entering" has our own implementation, as I haven't found a way to configure AgGrid to make behavior described in the task requirements. https://github.com/user-attachments/assets/c3d1083f-a7c1-40c8-b206-f70a0e7a825f # Important Notes The "quick" editing uncovered a problem with our AgGrid updates: the component's refreshing was callled way too often. In this PR it is addressed by reducing reactive dependencies of rowData and columnDefs, so they don't cause updates when only value changes (on value change, we refresh AgGrid by hand: this does not stop editing).
enso-org · Jan 30, 2025 · 5c07590 · 5c07590
1 parent 6a216f0
commit 5c07590
Show file tree

Hide file tree

Showing 9 changed files with 439 additions and 145 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,8 @@
 - [Tooltips are hidden when clicking on a button][12067].
 - [Fixed bug when clicking header in Table Editor Widget didn't start editing
   it][12064]
+- [When editing cells or header names in Table Editor Widget, `tab` and `enter`
+  keys jumps to next cell/ next row respectively.][12129]
 - [Fixed bugs occurring after renaming project from within graph editor][12106].
 
 [11889]: https://github.com/enso-org/enso/pull/11889
@@ -21,6 +23,7 @@
 [11908]: https://github.com/enso-org/enso/pull/11908
 [12067]: https://github.com/enso-org/enso/pull/12067
 [12064]: https://github.com/enso-org/enso/pull/12064
+[12129]: https://github.com/enso-org/enso/pull/12129
 [12106]: https://github.com/enso-org/enso/pull/12106
 
 #### Enso Standard Library

diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue
@@ -22,18 +22,16 @@ import { useToast } from '@/util/toast'
 import '@ag-grid-community/styles/ag-grid.css'
 import '@ag-grid-community/styles/ag-theme-alpine.css'
 import type {
-  CellEditingStartedEvent,
-  CellEditingStoppedEvent,
   ColDef,
-  Column,
   ColumnMovedEvent,
   ProcessDataFromClipboardParams,
   RowDragEndEvent,
 } from 'ag-grid-enterprise'
-import { ComponentInstance, computed, proxyRefs, ref } from 'vue'
+import { ComponentInstance, computed, ComputedRef, proxyRefs, ref, watch } from 'vue'
 import type { ComponentExposed } from 'vue-component-type-helpers'
 import { z } from 'zod'
 import TableHeader, { HeaderParams } from './WidgetTableEditor/TableHeader.vue'
+import { useTableEditHandler } from './WidgetTableEditor/editHandler'
 
 const props = defineProps(widgetProps(widgetDefinition))
 const graph = useGraphStore()
@@ -69,106 +67,36 @@ const { rowData, columnDefs, moveColumn, moveRow, pasteFromClipboard } = useTabl
   props.onUpdate,
 )
 
-// === Edit Handlers ===
-
-class CellEditing {
-  handler: WidgetEditHandler
-  editedCell: { rowIndex: number; colKey: Column<RowData> } | undefined
-  supressNextStopEditEvent: boolean = false
-
-  constructor() {
-    this.handler = WidgetEditHandler.New('WidgetTableEditor.cellEditHandler', props.input, {
-      cancel() {
-        grid.value?.gridApi?.stopEditing(true)
-      },
-      end() {
-        grid.value?.gridApi?.stopEditing(false)
-      },
-      suspend: () => {
-        return {
-          resume: () => this.editedCell && grid.value?.gridApi?.startEditingCell(this.editedCell),
-        }
-      },
-    })
-  }
+// Without this "cast" AgGridTableView gets confused when deducing its generic parameters.
+const columnDefsTyped: ComputedRef<ColDef<RowData>[]> = columnDefs
 
-  cellEditedInGrid(event: CellEditingStartedEvent) {
-    this.editedCell =
-      event.rowIndex != null ? { rowIndex: event.rowIndex, colKey: event.column } : undefined
-    if (!this.handler.isActive()) {
-      this.handler.start()
-    }
-  }
-
-  cellEditingStoppedInGrid(event: CellEditingStoppedEvent) {
-    if (!this.handler.isActive()) return
-    if (this.supressNextStopEditEvent && this.editedCell) {
-      this.supressNextStopEditEvent = false
-      // If row data changed, the editing will be stopped, but we want to continue it.
-      grid.value?.gridApi?.startEditingCell(this.editedCell)
-    } else {
-      this.handler.end()
-    }
-  }
-
-  rowDataChanged() {
-    if (this.handler.isActive()) {
-      this.supressNextStopEditEvent = true
-    }
-  }
-}
-
-const cellEditHandler = new CellEditing()
-
-class HeaderEditing {
-  handler: WidgetEditHandler
-  editedColId = ref<string>()
-  revertChangesCallback: (() => void) | undefined
+// === Edit Handlers ===
 
-  constructor() {
-    this.handler = WidgetEditHandler.New('WidgetTableEditor.headerEditHandler', props.input, {
-      cancel: () => {
-        this.revertChangesCallback?.()
-        this.editedColId.value = undefined
-      },
-      end: () => {
-        this.editedColId.value = undefined
-      },
+const { editedCell, gridEventHandlers, headerEventHandlers } = useTableEditHandler(
+  () => grid.value?.gridApi,
+  columnDefs,
+  (hooks) => {
+    const handler = WidgetEditHandler.New('WidgetTableEditor', props.input, {
+      ...hooks,
       pointerdown: (event) => {
         if (
           !(event.target instanceof HTMLInputElement) ||
           targetIsOutside(event, grid.value?.$el)
         ) {
-          this.handler.end()
+          handler.end()
         } else {
           return false
         }
       },
     })
-  }
-
-  headerEditedInGrid(colId: string, revertChanges: () => void) {
-    if (this.editedColId.value !== colId) {
-      this.editedColId.value = colId
-      if (!this.handler.isActive()) {
-        this.handler.start()
-      }
-    }
-    this.revertChangesCallback = revertChanges
-  }
-
-  headerEditingStoppedInGrid(colId: string) {
-    if (this.editedColId.value === colId) {
-      this.revertChangesCallback = undefined
-      this.editedColId.value = undefined
-      if (this.handler.isActive()) {
-        this.handler.end()
-      }
-    }
-  }
-}
+    return handler
+  },
+)
 
-const headerEditHandler = new HeaderEditing()
+watch(
+  () => props.input,
+  () => grid.value?.gridApi?.refreshCells(),
+)
 
 // === Resizing ===
 
@@ -238,9 +166,11 @@ function processDataFromClipboard({ data, api }: ProcessDataFromClipboardParams<
 // === Column Default Definition ===
 
 const headerComponentParams = proxyRefs({
-  editedColId: headerEditHandler.editedColId,
-  onHeaderEditingStarted: headerEditHandler.headerEditedInGrid.bind(headerEditHandler),
-  onHeaderEditingStopped: headerEditHandler.headerEditingStoppedInGrid.bind(headerEditHandler),
+  editedColId: computed(() =>
+    editedCell.value?.rowIndex === 'header' ? editedCell.value.colKey : undefined,
+  ),
+  onHeaderEditingStarted: headerEventHandlers.headerEditingStarted,
+  onHeaderEditingStopped: headerEventHandlers.headerEditingStopped,
 })
 
 const defaultColDef: ColDef<RowData> & {
@@ -284,7 +214,7 @@ export const widgetDefinition = defineWidget(
         ref="grid"
         class="inner"
         :defaultColDef="defaultColDef"
-        :columnDefs="columnDefs"
+        :columnDefs="columnDefsTyped"
         :rowData="rowData"
         :getRowId="(row) => `${row.data.index}`"
         :components="{
@@ -294,16 +224,13 @@ export const widgetDefinition = defineWidget(
         :suppressDragLeaveHidesColumns="true"
         :suppressMoveWhenColumnDragging="true"
         :processDataFromClipboard="processDataFromClipboard"
-        @keydown.enter.stop
+        v-on="gridEventHandlers"
         @keydown.arrow-left.stop
         @keydown.arrow-right.stop
         @keydown.arrow-up.stop
         @keydown.arrow-down.stop
         @keydown.backspace.stop
         @keydown.delete.stop
-        @cellEditingStarted="cellEditHandler.cellEditedInGrid($event)"
-        @cellEditingStopped="cellEditHandler.cellEditingStoppedInGrid($event)"
-        @rowDataUpdated="cellEditHandler.rowDataChanged()"
         @pointerdown.stop
         @click.stop
         @columnMoved="onColumnMoved"

diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue
@@ -1,6 +1,6 @@
 <script lang="ts">
 import SvgButton from '@/components/SvgButton.vue'
-import type { IHeaderParams } from 'ag-grid-community'
+import type { IHeaderParams } from 'ag-grid-enterprise'
 import { computed, ref, watch } from 'vue'
 
 /**
@@ -54,9 +54,11 @@ function emitEditEnd() {
   props.onHeaderEditingStopped?.(props.column.getColId())
 }
 
-watch(inputElement, (newVal, oldVal) => {
+watch(inputElement, (newVal) => {
   if (newVal != null) {
-    // Whenever input field appears, focus and select text
+    // Whenever input field appears, put text, focus and select
+    // We don't do that through props, because we don't want updates.
+    newVal.value = props.displayName
     newVal.focus()
     newVal.select()
   }
@@ -115,12 +117,14 @@ function onMouseRightClick(event: MouseEvent) {
         v-if="editing"
         ref="inputElement"
         class="ag-input-field-input ag-text-field-input"
-        :value="displayName"
-        @change="acceptNewName"
         @keydown.arrow-left.stop
         @keydown.arrow-right.stop
         @keydown.arrow-up.stop
         @keydown.arrow-down.stop
+        @keydown.tab.prevent="{
+          // We prevent default, because switching edit on tab is handled by the widget edit
+          // handlers
+        }"
       />
       <span
         v-else