feat: SURVEY-16594 UI enter and tab changes, fixes to grid sizing and…

… first row selected (#314) Enter will now save if invoked on input input[text] in a generic form Auto-resize not takes column headers into account Auto resize working on aggrid >28 Fixed selecting first index is selecting wrong index if sorting is applied. Removed onGridSizeChanged and onFirstDataRendered as they are no longer needed.
linz · Jun 1, 2023 · 8fdc6c6 · 8fdc6c6
1 parent 22d3362
commit 8fdc6c6
Show file tree

Hide file tree

Showing 9 changed files with 9,158 additions and 7,398 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -84,7 +84,7 @@
   "devDependencies": {
     "@rollup/plugin-commonjs": "^25.0.0",
     "@rollup/plugin-json": "^6.0.0",
-    "@rollup/plugin-node-resolve": "^15.0.2",
+    "@rollup/plugin-node-resolve": "^15.1.0",
     "@semantic-release/changelog": "^6.0.3",
     "@semantic-release/git": "^10.0.1",
     "@storybook/addon-essentials": "^6.5.16",
@@ -102,16 +102,16 @@
     "@testing-library/user-event": "^13.5.0",
     "@trivago/prettier-plugin-sort-imports": "^4.1.1",
     "@types/debounce-promise": "^3.1.6",
-    "@types/jest": "^29.5.1",
+    "@types/jest": "^29.5.2",
     "@types/lodash-es": "^4.17.7",
-    "@types/node": "^20.2.3",
-    "@types/react": "^17.0.59",
+    "@types/node": "^20.2.5",
+    "@types/react": "^17.0.60",
     "@types/react-dom": "^17.0.20",
     "@types/uuid": "^9.0.1",
-    "@typescript-eslint/parser": "^5.59.7",
+    "@typescript-eslint/parser": "^5.59.8",
     "babel-jest": "^29.5.0",
     "babel-preset-react-app": "^10.0.1",
-    "chromatic": "^6.17.4",
+    "chromatic": "^6.18.0",
     "conventional-changelog-conventionalcommits": "^5.0.0",
     "eslint": "^8.41.0",
     "eslint-config-prettier": "^8.8.0",
@@ -130,8 +130,8 @@
     "jest-expect-message": "^1.1.3",
     "mkdirp": "^3.0.1",
     "npm-run-all": "^4.1.5",
-    "postcss": "^8.4.23",
-    "postcss-loader": "^7.3.0",
+    "postcss": "^8.4.24",
+    "postcss-loader": "^7.3.2",
     "postcss-scss": "^4.0.6",
     "prettier": "^2.8.8",
     "react-app-polyfill": "^3.0.0",
@@ -142,11 +142,11 @@
     "rollup-plugin-postcss": "^4.0.2",
     "rollup-plugin-typescript2": "^0.34.1",
     "sass": "^1.62.1",
-    "sass-loader": "^13.3.0",
+    "sass-loader": "^13.3.1",
     "semantic-release": "^19.0.5",
     "style-loader": "^3.3.3",
     "stylelint": "^14.16.1",
-    "stylelint-config-prettier": "^9.0.4",
+    "stylelint-config-prettier": "^9.0.5",
     "stylelint-config-recommended-scss": "^8.0.0",
     "stylelint-config-standard": "^29.0.0",
     "stylelint-prettier": "3.0.0",

diff --git a/packageNotes.txt b/packageNotes.txt
@@ -1,3 +1,3 @@
-sass-loader is fixed at 10.3.1, anything higher breaks the build with getOptions() not found errors
-storybook-addon-mock is fixed at 2.4.1, otherwise you get blank pages. Probably a webpack 4->5 issue
-style-loader is fixed at 2.0.0 getOptions() issues again
+If you fix a package version, put the reason in here
+====================================================
+-empty-
diff --git a/src/components/Grid.tsx b/src/components/Grid.tsx
@@ -1,13 +1,7 @@
 import { CellClickedEvent, ColDef, ModelUpdatedEvent } from "ag-grid-community";
 import { CellClassParams, EditableCallback, EditableCallbackParams } from "ag-grid-community/dist/lib/entities/colDef";
 import { GridOptions } from "ag-grid-community/dist/lib/entities/gridOptions";
-import {
-  CellEvent,
-  FirstDataRenderedEvent,
-  GridReadyEvent,
-  GridSizeChangedEvent,
-  SelectionChangedEvent,
-} from "ag-grid-community/dist/lib/events";
+import { CellEvent, GridReadyEvent, SelectionChangedEvent } from "ag-grid-community/dist/lib/events";
 import { AgGridReact } from "ag-grid-react";
 import clsx from "clsx";
 import { difference, isEmpty, last, xorBy } from "lodash-es";
@@ -43,8 +37,6 @@ export interface GridProps {
   autoSelectFirstRow?: boolean;
   onColumnMoved?: GridOptions["onColumnMoved"];
   alwaysShowVerticalScroll?: boolean;
-  onGridSizeChanged?: GridOptions["onGridSizeChanged"];
-  onFirstDataRendered?: GridOptions["onFirstDataRendered"];
   suppressColumnVirtualization?: GridOptions["suppressColumnVirtualisation"];
   /**
    * When the grid is rendered using sizeColumns=="auto" this is called initially with the required container size to fit all content.
@@ -57,8 +49,8 @@ export interface GridProps {
    * <li>"fit" will adjust columns to fit within panel via min/max/initial sizing.
    * <b>Note:</b> This is only really needed if you have auto-height columns which prevents "auto" from working.
    * </li>
-   * <li>"auto" (default) will size columns based on their content but still obeying min/max sizing.</li>
-   * <li>"auto-skip-headers" same as auto but does not take headers into account.</li>
+   * <li>"auto" will size columns based on their content but still obeying min/max sizing.</li>
+   * <li>"auto-skip-headers" (default) same as auto but does not take headers into account.</li>
    * </ul>
    *
    * If you want to stretch to container width if width is greater than the container add a flex column.
@@ -82,6 +74,7 @@ export const Grid = ({
     setApis,
     prePopupOps,
     ensureRowVisible,
+    getFirstRowId,
     selectRowsById,
     focusByRowById,
     ensureSelectedRowIsVisible,
@@ -111,13 +104,9 @@ export const Grid = ({
     }
   }, [autoSizeAllColumns, params, sizeColumns, sizeColumnsToFit]);
 
-  const onFirstDataRendered = useCallback(
-    (event: FirstDataRenderedEvent) => {
-      params.onFirstDataRendered && params.onFirstDataRendered(event);
-      setInitialContentSize();
-    },
-    [params, setInitialContentSize],
-  );
+  useEffect(() => {
+    gridReady && setInitialContentSize();
+  }, [gridReady, setInitialContentSize]);
 
   /**
    * On data load select the first row of the grid if required.
@@ -127,7 +116,7 @@ export const Grid = ({
     if (!gridReady || hasSelectedFirstItem.current || !params.rowData || !externallySelectedItemsAreInSync) return;
     hasSelectedFirstItem.current = true;
     if (isNotEmpty(params.rowData) && isEmpty(params.externalSelectedItems)) {
-      const firstRowId = params.rowData[0].id;
+      const firstRowId = getFirstRowId();
       if (params.autoSelectFirstRow) {
         selectRowsById([firstRowId]);
       } else {
@@ -142,6 +131,7 @@ export const Grid = ({
     params.autoSelectFirstRow,
     params.rowData,
     selectRowsById,
+    getFirstRowId,
   ]);
 
   /**
@@ -360,13 +350,9 @@ export const Grid = ({
     [startCellEditing],
   );
 
-  const onGridSizeChanged = useCallback(
-    (event: GridSizeChangedEvent) => {
-      params.onGridSizeChanged && params.onGridSizeChanged(event);
-      sizeColumns !== "none" && sizeColumnsToFit();
-    },
-    [params, sizeColumns, sizeColumnsToFit],
-  );
+  const onGridSizeChanged = useCallback(() => {
+    sizeColumns !== "none" && sizeColumnsToFit();
+  }, [sizeColumns, sizeColumnsToFit]);
 
   /**
    * Once the grid has auto-sized we want to run fit to fit the grid in its container,
@@ -399,7 +385,6 @@ export const Grid = ({
           suppressRowClickSelection={true}
           rowSelection={rowSelection}
           suppressBrowserResizeObserver={true}
-          onFirstDataRendered={onFirstDataRendered}
           onGridSizeChanged={onGridSizeChanged}
           suppressColumnVirtualisation={suppressColumnVirtualization}
           suppressClickEdit={true}
@@ -418,9 +403,6 @@ export const Grid = ({
           postSortRows={params.postSortRows ?? postSortRows}
           onSelectionChanged={synchroniseExternalStateToGridSelection}
           onColumnMoved={params.onColumnMoved}
-          onColumnResized={() => {
-            sizeColumns !== "none" && sizeColumnsToFit();
-          }}
           alwaysShowVerticalScroll={params.alwaysShowVerticalScroll}
           isExternalFilterPresent={isExternalFilterPresent}
           doesExternalFilterPass={doesExternalFilterPass}

diff --git a/src/contexts/GridContext.tsx b/src/contexts/GridContext.tsx
@@ -25,6 +25,7 @@ export interface GridContextType<RowType extends GridBaseRow> {
   focusByRowById: (rowId: number) => void;
   ensureRowVisible: (id: number | string) => boolean;
   ensureSelectedRowIsVisible: () => void;
+  getFirstRowId: () => number;
   autoSizeAllColumns: (props?: { skipHeader?: boolean }) => { width: number } | null;
   sizeColumnsToFit: () => void;
   stopEditing: () => void;
@@ -112,6 +113,10 @@ export const GridContext = createContext<GridContextType<any>>({
   ensureSelectedRowIsVisible: () => {
     console.error("no context provider for ensureSelectedRowIsVisible");
   },
+  getFirstRowId: () => {
+    console.error("no context provider for getFirstRowId");
+    return -1;
+  },
   autoSizeAllColumns: () => {
     console.error("no context provider for autoSizeAllColumns");
     return null;

diff --git a/src/contexts/GridContextProvider.tsx b/src/contexts/GridContextProvider.tsx
@@ -74,6 +74,20 @@ export const GridContextProvider = <RowType extends GridBaseRow>(props: GridCont
     [gridApiOp],
   );
 
+  const getFirstRowId = useCallback((): number => {
+    let id = 0;
+    try {
+      gridApi?.forEachNodeAfterFilterAndSort((rowNode) => {
+        id = parseInt(rowNode.id ?? "0");
+        // this is the only way to get out of the loop
+        throw "expected exception - exit_loop";
+      });
+    } catch (ex) {
+      // ignore
+    }
+    return id;
+  }, [gridApi]);
+
   /**
    * Set the grid api when the grid is ready.
    */
@@ -330,13 +344,16 @@ export const GridContextProvider = <RowType extends GridBaseRow>(props: GridCont
   /**
    * Resize columns to fit container
    */
-  const autoSizeAllColumns = useCallback((): { width: number } | null => {
-    if (columnApi) {
-      columnApi.autoSizeAllColumns();
-      return { width: sumBy(columnApi.getColumnState(), "width") };
-    }
-    return null;
-  }, [columnApi]);
+  const autoSizeAllColumns = useCallback(
+    ({ skipHeader }): { width: number } | null => {
+      if (columnApi) {
+        columnApi.autoSizeAllColumns(skipHeader);
+        return { width: sumBy(columnApi.getColumnState(), "width") };
+      }
+      return null;
+    },
+    [columnApi],
+  );
 
   /**
    * Resize columns to fit container
@@ -530,6 +547,7 @@ export const GridContextProvider = <RowType extends GridBaseRow>(props: GridCont
         getFilteredSelectedRows,
         getSelectedRowIds,
         getFilteredSelectedRowIds,
+        getFirstRowId,
         editingCells,
         ensureRowVisible,
         ensureSelectedRowIsVisible,

diff --git a/src/react-menu3/components/ControlledMenu.tsx b/src/react-menu3/components/ControlledMenu.tsx
@@ -134,18 +134,8 @@ export const ControlledMenuFr = (
         saveButtonRef?.current?.click();
       };
 
+      // data-allowtabtosave is used such that list filter inputs can use tab to save
       const allowTabToSave = activeElement.getAttribute("data-allowtabtosave") == "true";
-      if (allowTabToSave && ev.key === "Tab") {
-        if (isDown) {
-          ev.preventDefault();
-          ev.stopPropagation();
-          lastTabDownEl.current = activeElement;
-        } else {
-          lastTabDownEl.current == activeElement &&
-            invokeSave(ev.shiftKey ? CloseReason.TAB_BACKWARD : CloseReason.TAB_FORWARD);
-        }
-        return;
-      }
 
       const inputElsIterator = thisDocument.querySelectorAll<HTMLElement>(".szh-menu--state-open input,textarea");
       let inputEls: HTMLElement[] = [];
@@ -154,48 +144,37 @@ export const ControlledMenuFr = (
       if (inputEls.length === 0) return;
       const firstInputEl = inputEls[0];
       const lastInputEl = inputEls[inputEls.length - 1];
-      if (activeElement !== firstInputEl && activeElement !== lastInputEl) return;
 
       const isTextArea = activeElement.nodeName === "TEXTAREA";
       const suppressEnterAutoSave = activeElement.getAttribute("data-disableenterautosave") == "true" || isTextArea;
 
-      switch (activeElement.nodeName) {
-        case "TEXTAREA":
-        case "INPUT": {
-          if ((activeElement === lastInputEl && activeElement === firstInputEl) || allowTabToSave) {
-            if (ev.key === "Tab") {
-              // Can't forward/backwards tab out of popup
-              ev.preventDefault();
-              ev.stopPropagation();
-              if (isDown) {
-                lastTabDownEl.current = activeElement;
-              } else {
-                lastTabDownEl.current == activeElement &&
-                  invokeSave(ev.shiftKey ? CloseReason.TAB_BACKWARD : CloseReason.TAB_FORWARD);
-              }
-            }
-            if (ev.key === "Enter" && !suppressEnterAutoSave) {
-              ev.preventDefault();
-              ev.stopPropagation();
-              if (isDown) {
-                lastEnterDownEl.current = activeElement;
-              } else {
-                lastEnterDownEl.current == activeElement && invokeSave(CloseReason.CLICK);
-              }
-            }
-          } else if (activeElement === lastInputEl) {
-            if (ev.key === "Tab" && !ev.shiftKey) {
-              // Can't backward tab out of popup
-              ev.preventDefault();
-              ev.stopPropagation();
+      if (ev.key === "Tab") {
+        const tabDirection = ev.shiftKey ? CloseReason.TAB_BACKWARD : CloseReason.TAB_FORWARD;
+        if (
+          (activeElement === lastInputEl && !ev.shiftKey) ||
+          (activeElement === firstInputEl && ev.shiftKey) ||
+          allowTabToSave
+        ) {
+          ev.preventDefault();
+          ev.stopPropagation();
 
-              if (isDown) {
-                lastTabDownEl.current = activeElement;
-              } else {
-                lastTabDownEl.current == activeElement && invokeSave(CloseReason.TAB_FORWARD);
-              }
-            }
-            if (ev.key === "Enter" && !suppressEnterAutoSave) {
+          if (isDown) {
+            lastTabDownEl.current = activeElement;
+          } else {
+            lastTabDownEl.current == activeElement && invokeSave(tabDirection);
+          }
+        }
+      }
+
+      const isTextInput =
+        "type" in activeElement &&
+        (activeElement.type === "text" || activeElement.type == null || activeElement.type === "textarea");
+
+      switch (activeElement.nodeName) {
+        case "INPUT":
+          {
+            // If there's only one input element, we support tab and enter
+            if (isTextInput && ev.key === "Enter" && !suppressEnterAutoSave) {
               ev.preventDefault();
               ev.stopPropagation();
               if (isDown) {
@@ -204,21 +183,8 @@ export const ControlledMenuFr = (
                 lastEnterDownEl.current == activeElement && invokeSave(CloseReason.CLICK);
               }
             }
-          } else if (activeElement === firstInputEl) {
-            if (ev.key === "Tab" && ev.shiftKey) {
-              // Can't backward tab out of popup
-              ev.preventDefault();
-              ev.stopPropagation();
-
-              if (isDown) {
-                lastTabDownEl.current = activeElement;
-              } else {
-                lastTabDownEl.current == activeElement && invokeSave(CloseReason.TAB_BACKWARD);
-              }
-            }
           }
           break;
-        }
       }
     },
     [anchorRef, saveButtonRef],

diff --git a/src/react-menu3/utils/utils.ts b/src/react-menu3/utils/utils.ts
@@ -91,15 +91,15 @@ export const indexOfNode = (nodeList: NodeListOf<Node>, node: Node) => findIndex
 export const focusFirstInput = (container: any) => {
   // We can't use instanceof Element in portals, so I use querySelectorAll as a proxy here
   if (!container || !("querySelectorAll" in container)) return false;
-  const inputs = container.querySelectorAll("input[type='text'],textarea");
+  const inputs = container.querySelectorAll("input[type='text'],input:not([type]),textarea");
   const input = inputs[0];
   // Using focus as proxy for HTMLElement
   if (!input || !("focus" in input)) return false;
   input.focus();
   // Text areas should start at end
   // this is a proxy for instanceof HTMLTextAreaElement
-  if (input.type === "textarea") {
-    input.selectionStart = input.value.length;
+  if (["textarea", "text"].includes(input.type)) {
+    input.setSelectionRange(0, input.value.length);
   }
   return true;
 };
diff --git a/src/stories/grid/interactions/GridKeyboardInteractions.stories.tsx b/src/stories/grid/interactions/GridKeyboardInteractions.stories.tsx
@@ -213,6 +213,9 @@ const GridKeyboardInteractionsTemplate: ComponentStory<typeof Grid> = (props: Gr
 
 export const GridKeyboardInteractions = GridKeyboardInteractionsTemplate.bind({});
 GridKeyboardInteractions.play = async ({ canvasElement }) => {
+  multiEditAction.mockReset();
+  eAction.mockReset();
+
   // Ensure first row/cell is selected on render
   await waitFor(async () => {
     const activeCell = canvasElement.ownerDocument.activeElement;