Bump AGGrid version (#11684)

* Bump AGGrid version + some fixes * Make styles in AgGridTableView global * CHANGELOG * Remove debug logs * Fix integration test * Fix unit test --------- Co-authored-by: James Dunkerley <[email protected]>
enso-org · Nov 28, 2024 · 6d9699c · 6d9699c
1 parent fe299b2
commit 6d9699c
Show file tree

Hide file tree

Showing 11 changed files with 127 additions and 112 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,9 +36,10 @@
   suitable type][11612].
 - [Visualizations on components are slightly transparent when not
   focused][11582].
-- [New design for vector-editing widget][11620]
+- [New design for vector-editing widget][11620].
 - [Default values on widgets are displayed in italic][11666].
-- [The `:` type operator can now be chained][11671]
+- [The `:` type operator can now be chained][11671].
+- [Fixed bug causing Table Visualization to show wrong data][11684].
 
 [11151]: https://github.com/enso-org/enso/pull/11151
 [11271]: https://github.com/enso-org/enso/pull/11271
@@ -58,12 +59,13 @@
 [11547]: https://github.com/enso-org/enso/pull/11547
 [11523]: https://github.com/enso-org/enso/pull/11523
 [11564]: https://github.com/enso-org/enso/pull/11564
+[11582]: https://github.com/enso-org/enso/pull/11582
 [11597]: https://github.com/enso-org/enso/pull/11597
 [11612]: https://github.com/enso-org/enso/pull/11612
-[11666]: https://github.com/enso-org/enso/pull/11666
-[11582]: https://github.com/enso-org/enso/pull/11582
 [11620]: https://github.com/enso-org/enso/pull/11620
+[11666]: https://github.com/enso-org/enso/pull/11666
 [11671]: https://github.com/enso-org/enso/pull/11671
+[11684]: https://github.com/enso-org/enso/pull/11684
 
 #### Enso Standard Library
 

diff --git a/app/gui/integration-test/project-view/tableVisualisation.spec.ts b/app/gui/integration-test/project-view/tableVisualisation.spec.ts
@@ -60,24 +60,16 @@ test('Copy from Table Visualization', async ({ page, context }) => {
   const widget = node.locator('.WidgetTableEditor')
   await expect(widget).toBeVisible()
   await widget.getByRole('button', { name: 'Add new column' }).click()
-  await widget.locator('.ag-cell', { hasNotText: /0/ }).first().click()
+  await widget.locator('.valueCell').first().click()
   await page.keyboard.press(`${CONTROL_KEY}+V`)
-  await expect(widget.locator('.ag-cell')).toHaveText([
-    '0',
+  await expect(widget.locator('.valueCell')).toHaveText([
     '0,0',
     '0,1',
-    '',
-    '1',
     '1,0',
     '1,1',
-    '',
-    '2',
     '2,0',
     '2,1',
     '',
-    '3',
-    '',
-    '',
     '',
   ])
 })
diff --git a/app/gui/integration-test/project-view/widgets.spec.ts b/app/gui/integration-test/project-view/widgets.spec.ts
@@ -626,7 +626,7 @@ test('Table widget', async ({ page }) => {
   await expect(widget.locator('.ag-text-field-input')).toHaveCount(0)
   await widget.locator('.ag-header-cell-text', { hasNotText: /#/ }).first().click()
   await expect(widget.locator('.ag-text-field-input')).toHaveCount(1)
-  await widget.locator('.ag-cell', { hasNotText: /0|1/ }).first().dblclick()
+  await widget.locator('.valueCell').first().dblclick()
   await expect(widget.locator('.ag-text-field-input')).toHaveCount(1)
   await widget.locator('.ag-header-cell-text', { hasNotText: /#/ }).first().click()
   await expect(widget.locator('.ag-text-field-input')).toHaveCount(1)

diff --git a/app/gui/package.json b/app/gui/package.json
@@ -79,11 +79,11 @@
     "validator": "^13.12.0",
     "zod": "^3.23.8",
     "zustand": "^4.5.4",
-    "@ag-grid-community/client-side-row-model": "^31.1.1",
-    "@ag-grid-community/core": "^31.1.1",
-    "@ag-grid-community/styles": "^31.1.1",
-    "@ag-grid-enterprise/core": "^31.1.1",
-    "@ag-grid-enterprise/range-selection": "^31.1.1",
+    "@ag-grid-community/client-side-row-model": "^32.3.3",
+    "@ag-grid-community/core": "^32.3.3",
+    "@ag-grid-community/styles": "^32.3.3",
+    "@ag-grid-enterprise/core": "^32.3.3",
+    "@ag-grid-enterprise/range-selection": "^32.3.3",
     "@babel/parser": "^7.24.7",
     "babel-plugin-react-compiler": "19.0.0-beta-a7bf2bd-20241110",
     "@codemirror/commands": "^6.6.0",
@@ -100,9 +100,9 @@
     "@noble/hashes": "^1.4.0",
     "@vueuse/core": "^10.4.1",
     "@vueuse/gesture": "^2.0.0",
-    "ag-grid-community": "^31.1.1",
-    "ag-grid-enterprise": "^31.1.1",
-    "ag-grid-vue3": "^31.1.1",
+    "ag-grid-community": "^32.3.3",
+    "ag-grid-enterprise": "^32.3.3",
+    "ag-grid-vue3": "^32.3.3",
     "codemirror": "^6.0.1",
     "culori": "^3.2.0",
     "events": "^3.3.0",

diff --git a/app/gui/playwright.config.ts b/app/gui/playwright.config.ts
@@ -67,7 +67,7 @@ export default defineConfig({
   fullyParallel: true,
   ...(WORKERS ? { workers: WORKERS } : {}),
   forbidOnly: isCI,
-  reporter: isCI ? ([['list'], ['blob']] as const) : ([['list']] as const),
+  reporter: isCI ? [['list'], ['blob']] : [['html']],
   retries: isCI ? 3 : 0,
   use: {
     headless: !DEBUG,
@@ -128,19 +128,6 @@ export default defineConfig({
         storageState: path.join(dirName, './playwright/.auth/user.json'),
       },
     },
-    {
-      name: 'Auth',
-      testDir: './integration-test/dashboard/auth',
-      expect: {
-        toHaveScreenshot: { threshold: 0 },
-        timeout: TIMEOUT_MS,
-      },
-      timeout: TIMEOUT_MS,
-      use: {
-        baseURL: `http://localhost:${ports.dashboard}`,
-        actionTimeout: TIMEOUT_MS,
-      },
-    },
     {
       name: 'Setup Tests for Project View',
       testMatch: /integration-test\/project-view\/setup\.ts/,

diff --git a/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue b/app/gui/src/project-view/components/GraphEditor/widgets/WidgetTableEditor.vue
@@ -253,7 +253,7 @@ export const widgetDefinition = defineWidget(
     <Suspense>
       <AgGridTableView
         ref="grid"
-        class="grid"
+        class="inner"
         :defaultColDef="defaultColDef"
         :columnDefs="columnDefs"
         :rowData="rowData"
@@ -292,8 +292,29 @@ export const widgetDefinition = defineWidget(
   position: relative;
 }
 
-.grid {
+.inner {
   width: 100%;
   height: 100%;
 }
+
+:deep(.newColumnCell) {
+  display: none;
+}
+
+:deep(.rowIndexCell) {
+  color: rgba(0, 0, 0, 0.4);
+}
+
+/* Those two classes are copied from AgGridTableView component.
+For some reason, Vue cannot load them there, probably because it is used also as Custom Element. */
+:deep(.inner) {
+  width: 100%;
+  height: 100%;
+}
+
+:deep(.ag-theme-alpine) {
+  --ag-grid-size: 3px;
+  --ag-list-item-height: 20px;
+  font-family: var(--font-mono);
+}
 </style>
diff --git a/...iew/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts b/...iew/components/GraphEditor/widgets/WidgetTableEditor/__tests__/tableInputArgument.test.ts
@@ -30,8 +30,8 @@ function generateTableOfOnes(rows: number, cols: number) {
   return ast
 }
 
-const expectedRowIndexColumnDef = { headerName: ROW_INDEX_HEADER }
-const expectedNewColumnDef = { cellStyle: { display: 'none' } }
+const expectedRowIndexColumnDef = { headerName: ROW_INDEX_HEADER, cellClass: 'rowIndexCell' }
+const expectedNewColumnDef = { cellClass: 'newColumnCell' }
 
 const CELLS_LIMIT_SQRT = Math.sqrt(CELLS_LIMIT)
 assert(CELLS_LIMIT_SQRT === Math.floor(CELLS_LIMIT_SQRT))

diff --git a/...i/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/tableInputArgument.ts b/...i/src/project-view/components/GraphEditor/widgets/WidgetTableEditor/tableInputArgument.ts
@@ -299,7 +299,7 @@ export function useTableInputArgument(
     mainMenuItems: ['autoSizeThis', 'autoSizeAll'],
     contextMenuItems: [removeRowMenuItem],
     lockPosition: 'right',
-    cellStyle: { display: 'none' },
+    cellClass: 'newColumnCell',
   }))
 
   const rowIndexColumnDef = computed<ColumnDef>(() => ({
@@ -314,7 +314,7 @@ export function useTableInputArgument(
     },
     mainMenuItems: ['autoSizeThis', 'autoSizeAll'],
     contextMenuItems: [removeRowMenuItem],
-    cellStyle: { color: 'rgba(0, 0, 0, 0.4)' },
+    cellClass: 'rowIndexCell',
     lockPosition: 'left',
     rowDrag: ({ data }: { data: RowData | undefined }) =>
       data?.index != null && data.index < rowCount.value,
@@ -377,6 +377,7 @@ export function useTableInputArgument(
             'separator',
             'export',
           ],
+          cellClass: 'valueCell',
         }) satisfies ColumnDef,
     )
     cols.unshift(rowIndexColumnDef.value)

diff --git a/app/gui/src/project-view/components/shared/AgGridTableView.vue b/app/gui/src/project-view/components/shared/AgGridTableView.vue
@@ -252,7 +252,7 @@ const { AgGridVue } = await import('ag-grid-vue3')
     <AgGridVue
       v-bind="$attrs"
       ref="grid"
-      class="ag-theme-alpine grid"
+      class="ag-theme-alpine inner"
       :headerHeight="26"
       :getRowHeight="getRowHeight"
       :rowData="rowData"
@@ -285,7 +285,7 @@ const { AgGridVue } = await import('ag-grid-vue3')
 <style src="@ag-grid-community/styles/ag-grid.css" />
 <style src="@ag-grid-community/styles/ag-theme-alpine.css" />
 <style scoped>
-.grid {
+.inner {
   width: 100%;
   height: 100%;
 }
@@ -295,8 +295,4 @@ const { AgGridVue } = await import('ag-grid-vue3')
   --ag-list-item-height: 20px;
   font-family: var(--font-mono);
 }
-
-.TableVisualization > .ag-theme-alpine > :deep(.ag-root-wrapper.ag-layout-normal) {
-  border-radius: 0 0 var(--radius-default) var(--radius-default);
-}
 </style>
diff --git a/app/gui/src/project-view/components/visualizations/TableVisualization.vue b/app/gui/src/project-view/components/visualizations/TableVisualization.vue
@@ -7,7 +7,7 @@ import { Pattern } from '@/util/ast/match'
 import { useVisualizationConfig } from '@/util/visualizationBuiltins'
 import type {
   CellClassParams,
-  CellClickedEvent,
+  CellDoubleClickedEvent,
   ColDef,
   ICellRendererParams,
   ITooltipParams,
@@ -369,20 +369,21 @@ function toField(
   const getSvgTemplate = (icon: string) =>
     `<svg viewBox="0 0 16 16" width="16" height="16"> <use xlink:href="${icons}#${icon}"/> </svg>`
   const svgTemplateWarning = showDataQuality ? getSvgTemplate('warning') : ''
-  const menu = `<span ref="eMenu" class="ag-header-icon ag-header-cell-menu-button"> </span>`
+  const menu = `<span data-ref="eMenu" class="ag-header-icon ag-header-cell-menu-button"> </span>`
+  const filterButton = `<span data-ref="eFilterButton" class="ag-header-icon ag-header-cell-filter-button" aria-hidden="true"></span>`
   const sort = `
-      <span ref="eFilter" class="ag-header-icon ag-header-label-icon ag-filter-icon" aria-hidden="true"></span>
-      <span ref="eSortOrder" class="ag-header-icon ag-sort-order" aria-hidden="true"></span>
-      <span ref="eSortAsc" class="ag-header-icon ag-sort-ascending-icon" aria-hidden="true"></span>
-      <span ref="eSortDesc" class="ag-header-icon ag-sort-descending-icon" aria-hidden="true"></span>
-      <span ref="eSortNone" class="ag-header-icon ag-sort-none-icon" aria-hidden="true"></span>
+      <span data-ref="eFilter" class="ag-header-icon ag-header-label-icon ag-filter-icon" aria-hidden="true"></span>
+      <span data-ref="eSortOrder" class="ag-header-icon ag-sort-order" aria-hidden="true"></span>
+      <span data-ref="eSortAsc" class="ag-header-icon ag-sort-ascending-icon" aria-hidden="true"></span>
+      <span data-ref="eSortDesc" class="ag-header-icon ag-sort-descending-icon" aria-hidden="true"></span>
+      <span data-ref="eSortNone" class="ag-header-icon ag-sort-none-icon" aria-hidden="true"></span>
     `
 
   const styles = 'display:flex; flex-direction:row; justify-content:space-between; width:inherit;'
   const template =
     icon ?
-      `<span style='${styles}'><span ref="eLabel" class="ag-header-cell-label" role="presentation" style='${styles}'> ${name} </span>${menu} ${sort} ${getSvgTemplate(icon)} ${svgTemplateWarning}</span>`
-    : `<span style='${styles}' ref="eLabel">${name} ${menu} ${sort} ${svgTemplateWarning}</span>`
+      `<span style='${styles}'><span data-ref="eLabel" class="ag-header-cell-label" role="presentation" style='${styles}'><span data-ref="eText" class="ag-header-cell-text"></span></span>${menu} ${filterButton} ${sort} ${getSvgTemplate(icon)} ${svgTemplateWarning}</span>`
+    : `<span style='${styles}' data-ref="eLabel"><span data-ref="eText" class="ag-header-cell-text"></span> ${menu} ${filterButton} ${sort} ${svgTemplateWarning}</span>`
 
   return {
     field: name,
@@ -421,7 +422,7 @@ function getAstPattern(selector?: string | number, action?: string) {
 }
 
 function createNode(
-  params: CellClickedEvent,
+  params: CellDoubleClickedEvent,
   selector: string,
   action?: string,
   castValueTypes?: string,
@@ -641,13 +642,12 @@ const getColumnValueToEnso = (columnName: string) => {
 
 function checkSortAndFilter(e: SortChangedEvent) {
   const gridApi = e.api
-  const columnApi = e.columnApi
-  if (gridApi == null || columnApi == null) {
+  if (gridApi == null) {
     console.warn('AG Grid column API does not exist.')
     isCreateNodeEnabled.value = false
     return
   }
-  const colState = columnApi.getColumnState()
+  const colState = gridApi.getColumnState()
   const filter = gridApi.getFilterModel()
   const sort = colState
     .map((cs) => {