feat: Column organization overflow and undo/redo (#2546)

Adds column order and visibilty overflow menu with show hidden columns
(for the visibility menu only) and undo/redo.

Moves in the menu or on the grid add to the undo/redo stack.
Hiding/unhiding adds to the undo/redo stack.
Creating a new group will add to the stack as a whole action when it is
named (moving columns + naming the group)

Unchecking the "Show hidden columns" option will hide the hidden
column/group entries in the menu. The setting is reset on menu open.

Author: mattrunyon

packages/code-studio/vite.config.ts

@@ -146,6 +146,9 @@ export default defineConfig(({ mode }) => {
     css: {
       devSourcemap: true,
     },
+    define: {
+      'process.env': {},
+    },
     plugins: [react()],
     esbuild: {
       /**

packages/components/src/shortcuts/GlobalShortcuts.ts

@@ -86,6 +86,20 @@ const GLOBAL_SHORTCUTS = {
     macShortcut: [MODIFIER.SHIFT, KEY.ENTER],
     isEditable: false,
   }),
+  UNDO: ShortcutRegistry.createAndAdd({
+    id: 'GLOBAL.UNDO',
+    name: 'Undo',
+    shortcut: [MODIFIER.CTRL, KEY.Z],
+    macShortcut: [MODIFIER.CMD, KEY.Z],
+    isEditable: false,
+  }),
+  REDO: ShortcutRegistry.createAndAdd({
+    id: 'GLOBAL.REDO',
+    name: 'Redo',
+    shortcut: [MODIFIER.CTRL, MODIFIER.SHIFT, KEY.Z],
+    macShortcut: [MODIFIER.CMD, MODIFIER.SHIFT, KEY.Z],
+    isEditable: false,
+  }),
 };
 
 export default GLOBAL_SHORTCUTS;

packages/components/src/spectrum/SpectrumMenu.scss

@@ -0,0 +1,16 @@
+// Override Bootstrap for Spectrum Menu items with keyboard shortcuts which we just re-export
+// Override for spectrum menu with keyboard shortcuts displayed
+[class*='spectrum-Menu'] {
+  kbd {
+    // Unsetting bootstrap overrides
+    padding: unset;
+    font-size: unset;
+    color: unset;
+    background-color: unset;
+    border-radius: unset;
+
+    // From Spectrum styles to match the label
+    padding-inline-start: var(--spectrum-global-dimension-size-125);
+    line-height: var(--spectrum-global-font-line-height-small, 1.3);
+  }
+}

packages/components/src/spectrum/SpectrumMenu.tsx

@@ -0,0 +1,4 @@
+import './SpectrumMenu.scss';
+
+// eslint-disable-next-line import/prefer-default-export
+export { Menu as SpectrumMenu } from '@adobe/react-spectrum';

packages/components/src/spectrum/collections.ts

@@ -5,7 +5,6 @@ export {
   // the Spectrum props type for upstream consumers that need to compose prop types.
   type SpectrumComboBoxProps,
   // ListBox - we aren't planning to support this component
-  Menu as SpectrumMenu,
   type SpectrumMenuProps,
   MenuTrigger,
   type SpectrumMenuTriggerProps as MenuTriggerProps,
@@ -20,3 +19,4 @@ export {
   TagGroup,
   type SpectrumTagGroupProps as TagGroupProps,
 } from '@adobe/react-spectrum';
+export { SpectrumMenu } from './SpectrumMenu';

packages/embed-widget/vite.config.ts

@@ -109,6 +109,9 @@ export default defineConfig(({ mode }) => {
         },
       },
     },
+    define: {
+      'process.env': {},
+    },
     optimizeDeps: {
       esbuildOptions: {
         // Some packages need this to start properly if they reference global

packages/grid/src/GridUtils.test.ts

@@ -88,9 +88,10 @@ describe('move items', () => {
   });
 
   it('skips moving an item to its original position', () => {
-    const movedItems = GridUtils.moveItem(2, 2, []);
+    const originalMoved: MoveOperation[] = [];
+    const movedItems = GridUtils.moveItem(2, 2, originalMoved);
 
-    expect(movedItems.length).toBe(0);
+    expect(movedItems).toBe(originalMoved);
     expectModelIndexes(movedItems, [0, 1, 2, 3]);
     expectVisibleIndexes(movedItems, [0, 1, 2, 3]);
   });
@@ -143,9 +144,10 @@ describe('move ranges', () => {
   });
 
   it('skips moving an item to its original position', () => {
-    const movedItems = GridUtils.moveRange([0, 2], 0, []);
+    const originalMoved: MoveOperation[] = [];
+    const movedItems = GridUtils.moveRange([0, 2], 0, originalMoved);
 
-    expect(movedItems.length).toBe(0);
+    expect(movedItems).toBe(originalMoved);
     expectModelIndexes(movedItems, [0, 1, 2, 3]);
     expectVisibleIndexes(movedItems, [0, 1, 2, 3]);
   });

packages/grid/src/GridUtils.ts

@@ -753,18 +753,31 @@ export class GridUtils {
    * @param from The visible index to move from
    * @param to The visible index to move the item to
    * @param oldMovedItems The old reordered items
-   * @returns The new reordered items
+   * @returns The new reordered items. The original array if the operation is a no-op.
    */
+  static moveItem(
+    from: VisibleIndex,
+    to: VisibleIndex,
+    oldMovedItems: MoveOperation[]
+  ): MoveOperation[];
+
   static moveItem(
     from: VisibleIndex,
     to: VisibleIndex,
     oldMovedItems: readonly MoveOperation[]
-  ): MoveOperation[] {
+  ): readonly MoveOperation[];
+
+  // The overloads are so we can return the original array if the operation is a no-op
+  static moveItem(
+    from: VisibleIndex,
+    to: VisibleIndex,
+    oldMovedItems: MoveOperation[] | readonly MoveOperation[]
+  ): MoveOperation[] | readonly MoveOperation[] {
     if (from === to) {
-      return [...oldMovedItems];
+      return oldMovedItems;
     }
 
-    const movedItems: MoveOperation[] = [...oldMovedItems];
+    const movedItems = [...oldMovedItems];
     const lastMovedItem = movedItems[movedItems.length - 1];
 
     // Check if we should combine with the previous move
@@ -806,14 +819,29 @@ export class GridUtils {
    *                    E.g. Move range [0, 2] 1 item down (after element 3)
    *                    The move is [0, 2] -> 1 if this is false. [0, 2] -> 3 if this is true
    *                    Both will result in [0, 2] -> 1
-   * @returns The new reordered items
+   * @returns The new reordered items. The original array if the operation is a no-op.
    */
+  static moveRange(
+    from: BoundedAxisRange,
+    to: VisibleIndex,
+    oldMovedItems: MoveOperation[],
+    isPreMoveTo?: boolean
+  ): MoveOperation[];
+
   static moveRange(
     from: BoundedAxisRange,
     toParam: VisibleIndex,
     oldMovedItems: readonly MoveOperation[],
+    isPreMoveTo?: boolean
+  ): readonly MoveOperation[];
+
+  // The overloads are so we can return the original array if the operation is a no-op
+  static moveRange(
+    from: BoundedAxisRange,
+    toParam: VisibleIndex,
+    oldMovedItems: MoveOperation[] | readonly MoveOperation[],
     isPreMoveTo = false
-  ): MoveOperation[] {
+  ): MoveOperation[] | readonly MoveOperation[] {
     if (from[0] === from[1]) {
       return GridUtils.moveItem(from[0], toParam, oldMovedItems);
     }
@@ -825,7 +853,7 @@ export class GridUtils {
     }
 
     if (from[0] === to) {
-      return [...oldMovedItems];
+      return oldMovedItems;
     }
 
     const movedItems: MoveOperation[] = [...oldMovedItems];
@@ -863,8 +891,23 @@ export class GridUtils {
     from: VisibleIndex | BoundedAxisRange,
     to: VisibleIndex,
     oldMovedItems: MoveOperation[],
+    isPreMoveTo?: boolean
+  ): MoveOperation[];
+
+  static moveItemOrRange(
+    from: VisibleIndex | BoundedAxisRange,
+    to: VisibleIndex,
+    oldMovedItems: readonly MoveOperation[],
+    isPreMoveTo?: boolean
+  ): readonly MoveOperation[];
+
+  // The overloads are so we can return the original array if the operation is a no-op
+  static moveItemOrRange(
+    from: VisibleIndex | BoundedAxisRange,
+    to: VisibleIndex,
+    oldMovedItems: MoveOperation[] | readonly MoveOperation[],
     isPreMoveTo = false
-  ): MoveOperation[] {
+  ): MoveOperation[] | readonly MoveOperation[] {
     return Array.isArray(from)
       ? GridUtils.moveRange(from, to, oldMovedItems, isPreMoveTo)
       : GridUtils.moveItem(from, to, oldMovedItems);

packages/grid/src/mouse-handlers/GridColumnMoveMouseHandler.ts

@@ -680,7 +680,7 @@ class GridColumnMoveMouseHandler extends GridMouseHandler {
     draggingColumn: ColumnInfo,
     to: number,
     movedColumns: readonly MoveOperation[]
-  ): MoveOperation[] {
+  ): readonly MoveOperation[] {
     const newMovedColumns = draggingColumn.isColumnGroup
       ? GridUtils.moveRange(draggingColumn.range, to, movedColumns)
       : GridUtils.moveItem(draggingColumn.visibleIndex, to, movedColumns);

packages/iris-grid/src/ColumnHeaderGroup.ts

@@ -91,4 +91,13 @@ export default class ColumnHeaderGroup implements IColumnHeaderGroup {
   get isNew(): boolean {
     return this.name.startsWith(ColumnHeaderGroup.NEW_GROUP_PREFIX);
   }
+
+  /**
+   * Checks if a group is valid.
+   * An invalid group needs to be re-parsed with the corresponding model.
+   * @returns true if the group is valid (all children have indexes)
+   */
+  isValid(): boolean {
+    return this.children.length === this.childIndexes.length;
+  }
 }