From 991c0eeba71e3cef8e2b0466066ccbc23b4f9bf9 Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Tue, 7 Oct 2025 18:40:48 -0500 Subject: [PATCH 01/12] feat: Add overflow menu with undo/redo to column order menu --- packages/code-studio/vite.config.ts | 3 + .../src/shortcuts/GlobalShortcuts.ts | 14 + .../components/src/spectrum/SpectrumMenu.scss | 16 + .../components/src/spectrum/SpectrumMenu.tsx | 4 + .../components/src/spectrum/collections.ts | 2 +- packages/embed-widget/vite.config.ts | 3 + packages/grid/src/GridUtils.test.ts | 10 +- packages/grid/src/GridUtils.ts | 28 +- .../GridColumnMoveMouseHandler.ts | 2 +- packages/iris-grid/src/IrisGrid.tsx | 22 +- .../visibility-ordering-builder/SearchBar.tsx | 41 +++ .../VisibilityOrderingBuilder.test.tsx | 4 +- .../VisibilityOrderingBuilder.tsx | 279 +++++++++++++++++- .../VisibilityOrderingBuilderUtils.test.ts | 38 ++- .../VisibilityOrderingBuilderUtils.ts | 12 +- .../VisibilityOrderingGroup.tsx | 19 +- packages/react-hooks/src/index.ts | 1 + packages/react-hooks/src/useUndoRedo.test.ts | 87 ++++++ packages/react-hooks/src/useUndoRedo.ts | 107 +++++++ 19 files changed, 630 insertions(+), 62 deletions(-) create mode 100644 packages/components/src/spectrum/SpectrumMenu.scss create mode 100644 packages/components/src/spectrum/SpectrumMenu.tsx create mode 100644 packages/iris-grid/src/sidebar/visibility-ordering-builder/SearchBar.tsx create mode 100644 packages/react-hooks/src/useUndoRedo.test.ts create mode 100644 packages/react-hooks/src/useUndoRedo.ts diff --git a/packages/code-studio/vite.config.ts b/packages/code-studio/vite.config.ts index 4d12c46419..9fda2f34f7 100644 --- a/packages/code-studio/vite.config.ts +++ b/packages/code-studio/vite.config.ts @@ -146,6 +146,9 @@ export default defineConfig(({ mode }) => { css: { devSourcemap: true, }, + define: { + 'process.env': {}, + }, plugins: [react()], esbuild: { /** diff --git a/packages/components/src/shortcuts/GlobalShortcuts.ts b/packages/components/src/shortcuts/GlobalShortcuts.ts index 16376841ed..2d45eefafd 100644 --- a/packages/components/src/shortcuts/GlobalShortcuts.ts +++ b/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; diff --git a/packages/components/src/spectrum/SpectrumMenu.scss b/packages/components/src/spectrum/SpectrumMenu.scss new file mode 100644 index 0000000000..7c93f523b9 --- /dev/null +++ b/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: revert; + color: revert; + background-color: revert; + border-radius: revert; + + // 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); + } +} diff --git a/packages/components/src/spectrum/SpectrumMenu.tsx b/packages/components/src/spectrum/SpectrumMenu.tsx new file mode 100644 index 0000000000..e696adfed6 --- /dev/null +++ b/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'; diff --git a/packages/components/src/spectrum/collections.ts b/packages/components/src/spectrum/collections.ts index 200ab06002..4ebeba426f 100644 --- a/packages/components/src/spectrum/collections.ts +++ b/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'; diff --git a/packages/embed-widget/vite.config.ts b/packages/embed-widget/vite.config.ts index 16c8c2ed84..4d893bba39 100644 --- a/packages/embed-widget/vite.config.ts +++ b/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 diff --git a/packages/grid/src/GridUtils.test.ts b/packages/grid/src/GridUtils.test.ts index d1f80b5e9b..34e59056ec 100644 --- a/packages/grid/src/GridUtils.test.ts +++ b/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]); }); diff --git a/packages/grid/src/GridUtils.ts b/packages/grid/src/GridUtils.ts index e6f7e1c5ce..21d59de37d 100644 --- a/packages/grid/src/GridUtils.ts +++ b/packages/grid/src/GridUtils.ts @@ -755,16 +755,16 @@ export class GridUtils { * @param oldMovedItems The old reordered items * @returns The new reordered items */ - static moveItem( + static moveItem( from: VisibleIndex, to: VisibleIndex, - oldMovedItems: readonly MoveOperation[] - ): MoveOperation[] { + oldMovedItems: T + ): T { 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 @@ -787,7 +787,7 @@ export class GridUtils { movedItems.push({ from, to }); } - return movedItems; + return movedItems as unknown as T; } /** @@ -808,12 +808,12 @@ export class GridUtils { * Both will result in [0, 2] -> 1 * @returns The new reordered items */ - static moveRange( + static moveRange( from: BoundedAxisRange, toParam: VisibleIndex, - oldMovedItems: readonly MoveOperation[], + oldMovedItems: T, isPreMoveTo = false - ): MoveOperation[] { + ): T { if (from[0] === from[1]) { return GridUtils.moveItem(from[0], toParam, oldMovedItems); } @@ -825,7 +825,7 @@ export class GridUtils { } if (from[0] === to) { - return [...oldMovedItems]; + return oldMovedItems; } const movedItems: MoveOperation[] = [...oldMovedItems]; @@ -856,15 +856,15 @@ export class GridUtils { movedItems.pop(); } - return movedItems; + return movedItems as unknown as T; } - static moveItemOrRange( + static moveItemOrRange( from: VisibleIndex | BoundedAxisRange, to: VisibleIndex, - oldMovedItems: MoveOperation[], + oldMovedItems: T, isPreMoveTo = false - ): MoveOperation[] { + ): T { return Array.isArray(from) ? GridUtils.moveRange(from, to, oldMovedItems, isPreMoveTo) : GridUtils.moveItem(from, to, oldMovedItems); diff --git a/packages/grid/src/mouse-handlers/GridColumnMoveMouseHandler.ts b/packages/grid/src/mouse-handlers/GridColumnMoveMouseHandler.ts index c728e8f34a..bd7d74b671 100644 --- a/packages/grid/src/mouse-handlers/GridColumnMoveMouseHandler.ts +++ b/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); diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 353f90a3b4..62db4bbed3 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/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 { 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; @@ -2584,6 +2587,15 @@ class IrisGrid extends Component { }); } + /** + * 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 @@ -3401,12 +3413,13 @@ class IrisGrid extends Component { columnHeaderGroups: readonly (DhType.ColumnGroup | ColumnHeaderGroup)[] ): void { const { model } = this.props; + this.setState( { - columnHeaderGroups: IrisGridUtils.parseColumnHeaderGroups( - model, - columnHeaderGroups - ).groups, + columnHeaderGroups: columnHeaderGroups.every(isColumnHeaderGroup) + ? columnHeaderGroups + : IrisGridUtils.parseColumnHeaderGroups(model, columnHeaderGroups) + .groups, }, () => this.grid?.forceUpdate() ); @@ -4922,6 +4935,7 @@ class IrisGrid extends Component { onReset={this.handleColumnVisibilityReset} onMovedColumnsChanged={this.handleMovedColumnsChanged} onColumnHeaderGroupChanged={this.handleHeaderGroupsChanged} + onFrozenColumnsChanged={this.handleFrozenColumnsChanged} key={OptionType.VISIBILITY_ORDERING_BUILDER} /> ); diff --git a/packages/iris-grid/src/sidebar/visibility-ordering-builder/SearchBar.tsx b/packages/iris-grid/src/sidebar/visibility-ordering-builder/SearchBar.tsx new file mode 100644 index 0000000000..03527e7615 --- /dev/null +++ b/packages/iris-grid/src/sidebar/visibility-ordering-builder/SearchBar.tsx @@ -0,0 +1,41 @@ +import { useRef, useState } from 'react'; +import { Dialog, DialogTrigger, Popover } from 'react-aria-components'; +import type { TextFieldRef } from '@react-types/textfield'; +import { + Content, + // Dialog, + DialogContainer, + Popper, + // DialogTrigger, + SearchField, + Text, +} from '@deephaven/components'; + +interface SearchBarProps { + items: { name: string; movable: boolean }[]; +} + +export function SearchBar({ items }: SearchBarProps): JSX.Element { + const [isModalOpen, setIsModalOpen] = useState(false); + const searchRef = useRef(null); + return ( + <> + setIsModalOpen(true)} + aria-label="Search columns" + /> + + Testing + + + ); +} + +export default SearchBar; diff --git a/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.test.tsx b/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.test.tsx index b5d7dbe26e..c035d12c11 100644 --- a/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.test.tsx +++ b/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.test.tsx @@ -70,7 +70,7 @@ function Builder({ onColumnVisibilityChanged={onColumnVisibilityChanged} onMovedColumnsChanged={onMovedColumnsChanged} onReset={onReset} - ref={builderRef} + __testRef={builderRef} /> ); } @@ -1012,7 +1012,7 @@ test('Edit group name', async () => { await user.type(nameInput, '{Backspace}'); expect(screen.queryAllByText('Invalid name').length).toBe(0); - const confirmButton = screen.getByLabelText('Confirm'); + const confirmButton = await screen.findByLabelText('Confirm'); await user.click(confirmButton); expect(mockHandler).toBeCalledWith([ expect.objectContaining({ ...NESTED_COLUMN_HEADER_GROUPS[1], name: 'abc' }), diff --git a/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.tsx b/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.tsx index de3f3eaabf..e746b49288 100644 --- a/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.tsx +++ b/packages/iris-grid/src/sidebar/visibility-ordering-builder/VisibilityOrderingBuilder.tsx @@ -1,6 +1,10 @@ import React, { + memo, type ChangeEvent, PureComponent, + useCallback, + useEffect, + useRef, type ReactElement, } from 'react'; import classNames from 'classnames'; @@ -25,13 +29,30 @@ import { vsRefresh, vsCircleLargeFilled, vsAdd, + vsBlank, + vsCheck, + vsKebabVertical, } from '@deephaven/icons'; import type { dh } from '@deephaven/jsapi-types'; import memoize from 'memoizee'; import debounce from 'lodash.debounce'; -import { Button, SearchInput } from '@deephaven/components'; +import type { Key } from '@react-types/shared'; +import { + ActionButton, + Button, + GLOBAL_SHORTCUTS, + Icon, + Item, + Keyboard, + MenuTrigger, + SearchInput, + Section, + SpectrumMenu, + Text, +} from '@deephaven/components'; import clamp from 'lodash.clamp'; import throttle from 'lodash.throttle'; +import { useUndoRedo } from '@deephaven/react-hooks'; import './VisibilityOrderingBuilder.scss'; import { type DisplayColumn } from '../../IrisGridModel'; import type IrisGridModel from '../../IrisGridModel'; @@ -58,7 +79,7 @@ interface IndexRange { nextIndex: number; } -interface VisibilityOrderingBuilderProps { +interface VisibilityOrderingBuilderWrapperProps { model: IrisGridModel; movedColumns: readonly MoveOperation[]; hiddenColumns: readonly ModelIndex[]; @@ -75,6 +96,21 @@ interface VisibilityOrderingBuilderProps { onColumnHeaderGroupChanged: ( groups: readonly (dh.ColumnGroup | ColumnHeaderGroup)[] ) => void; + onFrozenColumnsChanged: (columns: readonly ColumnName[]) => void; + __testRef?: React.Ref; +} + +interface VisibilityOrderingBuilderProps + extends Omit< + VisibilityOrderingBuilderWrapperProps, + 'onFrozenColumnsChanged' | '__testRef' + > { + undo: () => void; + redo: () => void; + canUndo: boolean; + canRedo: boolean; + startUndoGroup: () => void; + endUndoGroup: () => void; } interface VisibilityOrderingBuilderState { @@ -84,6 +120,7 @@ interface VisibilityOrderingBuilderState { prevQueriedColumns: Record | undefined; lastSelectedColumn: string; searchFilter: string; + showHiddenColumns: boolean; } class VisibilityOrderingBuilder extends PureComponent< @@ -117,6 +154,8 @@ class VisibilityOrderingBuilder extends PureComponent< this.validateGroupName = this.validateGroupName.bind(this); this.addColumnToSelected = this.addColumnToSelected.bind(this); this.handleDragStart = this.handleDragStart.bind(this); + this.handleOverflowAction = this.handleOverflowAction.bind(this); + this.handleKeyboardShortcut = this.handleKeyboardShortcut.bind(this); this.state = { selectedColumns: new Set(), @@ -125,6 +164,7 @@ class VisibilityOrderingBuilder extends PureComponent< prevQueriedColumns: undefined, lastSelectedColumn: '', searchFilter: '', + showHiddenColumns: true, }; this.list = null; @@ -140,6 +180,13 @@ class VisibilityOrderingBuilder extends PureComponent< componentWillUnmount(): void { this.debouncedSearchColumns.cancel(); + + const { columnHeaderGroups, onColumnHeaderGroupChanged } = this.props; + // Clean up unnamed groups on unmount + const newGroups = columnHeaderGroups.filter(group => !group.isNew); + if (newGroups.length !== columnHeaderGroups.length) { + onColumnHeaderGroupChanged(newGroups); + } } list: HTMLDivElement | null; @@ -857,7 +904,8 @@ class VisibilityOrderingBuilder extends PureComponent< } handleGroupNameChange(group: ColumnHeaderGroup, newName: string): void { - const { columnHeaderGroups, onColumnHeaderGroupChanged } = this.props; + const { columnHeaderGroups, onColumnHeaderGroupChanged, endUndoGroup } = + this.props; const newGroups = [...columnHeaderGroups]; const oldName = group.name; @@ -881,6 +929,7 @@ class VisibilityOrderingBuilder extends PureComponent< } onColumnHeaderGroupChanged(newGroups); + endUndoGroup(); } handleGroupColorChange = throttle( @@ -914,8 +963,12 @@ class VisibilityOrderingBuilder extends PureComponent< } handleGroupCreate(): void { - const { movedColumns, onMovedColumnsChanged, onColumnHeaderGroupChanged } = - this.props; + const { + movedColumns, + onMovedColumnsChanged, + onColumnHeaderGroupChanged, + startUndoGroup, + } = this.props; const { newMoves, groups } = this.moveSelectedColumns( VisibilityOrderingBuilder.MOVE_OPTIONS.TOP @@ -941,12 +994,12 @@ class VisibilityOrderingBuilder extends PureComponent< childIndexes: [...new Set(childIndexes)], // Remove any duplicates }); + startUndoGroup(); + onMovedColumnsChanged(movedColumns.concat(newMoves), () => { this.list?.parentElement?.scroll({ top: 0 }); }); - onColumnHeaderGroupChanged(newGroups.concat([newGroup])); - this.resetSelection(); } @@ -990,6 +1043,7 @@ class VisibilityOrderingBuilder extends PureComponent< return ( { const { movedColumns } = this.props; @@ -1139,11 +1194,15 @@ class VisibilityOrderingBuilder extends PureComponent< : data.visibleIndex === lastMovableIndex ); - const movableItems = treeItems.slice( + let movableItems = treeItems.slice( firstMovableTreeIndex, lastMovableTreeIndex + 1 ); + if (!showHiddenColumns) { + movableItems = movableItems.filter(item => item.data.isVisible); + } + // No movable items. Render all as immovable if (firstMovableIndex == null || lastMovableIndex === null) { for ( @@ -1223,13 +1282,46 @@ class VisibilityOrderingBuilder extends PureComponent< { max: 1000 } ); + handleOverflowAction(key: Key): void { + const { undo, redo } = this.props; + switch (key) { + case 'undo': + undo(); + break; + case 'redo': + redo(); + break; + case 'showHidden': + this.setState(prev => ({ showHiddenColumns: !prev.showHiddenColumns })); + break; + } + } + + handleKeyboardShortcut(event: React.KeyboardEvent): void { + const { canUndo, canRedo, undo, redo } = this.props; + if (GLOBAL_SHORTCUTS.UNDO.matchesEvent(event) && canUndo) { + event.preventDefault(); + undo(); + } else if (GLOBAL_SHORTCUTS.REDO.matchesEvent(event) && canRedo) { + event.preventDefault(); + redo(); + } + } + render(): ReactElement { - const { model, hiddenColumns, onColumnVisibilityChanged } = this.props; + const { + model, + hiddenColumns, + onColumnVisibilityChanged, + canUndo, + canRedo, + } = this.props; const { selectedColumns, searchFilter, prevQueriedColumns, queriedColumnIndex, + showHiddenColumns, } = this.state; const hasSelection = selectedColumns.size > 0; const treeItems = this.getTreeItems(); @@ -1261,7 +1353,8 @@ class VisibilityOrderingBuilder extends PureComponent< const visibilityOrderingList = this.makeVisibilityOrderingList( model.columns, - treeItems + treeItems, + showHiddenColumns ); const cursor = { @@ -1273,7 +1366,12 @@ class VisibilityOrderingBuilder extends PureComponent< const matchCount = Object.keys(prevQueriedColumns ?? {}).length; return ( -
+