feat: Column organization overflow and undo/redo

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;
+  }
 }

packages/iris-grid/src/IrisGrid.tsx

@@ -197,6 +197,7 @@ import type ColumnHeaderGroup from './ColumnHeaderGroup';
 import { IrisGridThemeContext } from './IrisGridThemeProvider';
 import { isMissingPartitionError } from './MissingPartitionError';
 import { NoPastePermissionModal } from './NoPastePermissionModal';
+import { isColumnHeaderGroup } from './ColumnHeaderGroup';
 
 const log = Log.module('IrisGrid');
 
@@ -645,6 +646,8 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     this.handleViewportUpdated = this.handleViewportUpdated.bind(this);
     this.makeQuickFilter = this.makeQuickFilter.bind(this);
     this.setFilterMap = this.setFilterMap.bind(this);
+    this.handleFrozenColumnsChanged =
+      this.handleFrozenColumnsChanged.bind(this);
 
     this.grid = null;
     this.lastLoadedConfig = null;
@@ -2588,6 +2591,15 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     });
   }
 
+  /**
+   * Updates the entire list of frozen columns.
+   * Used by VisibilityOrderingBuilder.
+   * @param frozenColumns The new list of frozen columns
+   */
+  handleFrozenColumnsChanged(frozenColumns: readonly ColumnName[]): void {
+    this.setState({ frozenColumns });
+  }
+
   toggleExpandColumn(
     modelIndex: ModelIndex,
     expandDescendants?: boolean
@@ -3405,12 +3417,22 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     columnHeaderGroups: readonly (DhType.ColumnGroup | ColumnHeaderGroup)[]
   ): void {
     const { model } = this.props;
+    const { columnHeaderGroups: prevColumnHeaderGroups } = this.state;
+    if (prevColumnHeaderGroups === columnHeaderGroups) {
+      return;
+    }
+
     this.setState(
       {
-        columnHeaderGroups: IrisGridUtils.parseColumnHeaderGroups(
-          model,
-          columnHeaderGroups
-        ).groups,
+        // undo/redo will pass already parsed groups
+        // Parsing them again causes a loop with undo/redo that makes it unusable
+        columnHeaderGroups: columnHeaderGroups.every(
+          (group): group is ColumnHeaderGroup =>
+            isColumnHeaderGroup(group) && group.isValid()
+        )
+          ? columnHeaderGroups
+          : IrisGridUtils.parseColumnHeaderGroups(model, columnHeaderGroups)
+              .groups,
       },
       () => this.grid?.forceUpdate()
     );
@@ -4926,6 +4948,7 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
               onReset={this.handleColumnVisibilityReset}
               onMovedColumnsChanged={this.handleMovedColumnsChanged}
               onColumnHeaderGroupChanged={this.handleHeaderGroupsChanged}
+              onFrozenColumnsChanged={this.handleFrozenColumnsChanged}
               key={OptionType.VISIBILITY_ORDERING_BUILDER}
             />
           );