Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB 6296 Delete row shortcut not working #3340

Merged
merged 9 commits into from
Mar 28, 2025
Merged

CB 6296 Delete row shortcut not working #3340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
efeae45
CB-6296 fix: delete keystroke marks selected row for deletion
SychevAndrey Mar 25, 2025
8302800
CB-6296 fix: focus first element on load, remove extra handler
SychevAndrey Mar 25, 2025
7769a8e
Merge branch 'devel' into CB-6296-Delete-row-shortcut-not-working
EvgeniaBzzz Mar 26, 2025
8aa53e2
CB-6286 refactor: change onCellKeyDown handler interface inside DataG…
SychevAndrey Mar 27, 2025
d972d67
Merge branch 'devel' into CB-6296-Delete-row-shortcut-not-working
SychevAndrey Mar 27, 2025
26cbdab
CB-6296 refactor: set initial focus differently
SychevAndrey Mar 27, 2025
024021c
CB-6296 refactor: remove manual scroll control
SychevAndrey Mar 28, 2025
a7465dd
refactor: don't expose inner data-grid type
SychevAndrey Mar 28, 2025
b034200
Merge branch 'devel' into CB-6296-Delete-row-shortcut-not-working
Wroud Mar 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions common-react/@dbeaver/react-data-grid/src/DataGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export interface ICellPosition {
colIdx: number;
}

export interface DataGridCellKeyboardEvent extends React.KeyboardEvent<HTMLDivElement> {
preventGridDefault: () => void;
isGridDefaultPrevented: () => boolean;
}

export interface DataGridProps extends IDataGridCellContext, IDataGridRowContext, IDataGridHeaderCellContext, React.PropsWithChildren {
getRowHeight?: (rowIdx: number) => number;
getRowId?: (rowIdx: number) => React.Key;
Expand All @@ -26,6 +31,7 @@ export interface DataGridProps extends IDataGridCellContext, IDataGridRowContext
onScroll?: (event: React.UIEvent<HTMLDivElement>) => void;
onFocus?: (position: ICellPosition) => void;
onEditorOpen?: (position: ICellPosition) => void;
onCellKeyDown?: (position: ICellPosition, event: DataGridCellKeyboardEvent) => void;
className?: string;
}

Expand Down Expand Up @@ -59,6 +65,7 @@ export const DataGrid = forwardRef<DataGridRef, DataGridProps>(function DataGrid
onCellChange,
children,
className,
onCellKeyDown,
},
ref,
) {
Expand Down Expand Up @@ -135,6 +142,7 @@ export const DataGrid = forwardRef<DataGridRef, DataGridProps>(function DataGrid
rowHeight={getRowHeight ? row => getRowHeight(row.idx) : undefined}
rowKeyGetter={getRowId ? row => getRowId(row.idx) : undefined}
onSelectedCellChange={handleCellFocus}
onCellKeyDown={onCellKeyDown ? (args, event) => onCellKeyDown({ rowIdx: args.rowIdx, colIdx: args.column.idx }, event) : undefined}
renderers={{
renderRow: rowRenderer,
renderCell: cellRenderer,
Expand Down
3 changes: 2 additions & 1 deletion webapp/packages/plugin-data-grid/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
* Copyright (C) 2020-2025 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
Expand All @@ -20,4 +20,5 @@ export {
type ICellPosition,
type IDataGridCellRenderer,
type IDataGridCellProps,
type DataGridProps,
} from '@dbeaver/react-data-grid';
195 changes: 64 additions & 131 deletions webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/*
* CloudBeaver - Cloud Database Manager
* Copyright (C) 2020-2024 DBeaver Corp and others
* Copyright (C) 2020-2025 DBeaver Corp and others
*
* Licensed under the Apache License, Version 2.0.
* you may not use this file except in compliance with the License.
*/
import { observer } from 'mobx-react-lite';
import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, type HTMLAttributes } from 'react';
import { useCallback, useLayoutEffect, useMemo, useRef, type HTMLAttributes } from 'react';
import { reaction } from 'mobx';

import { s, TextPlaceholder, useObjectRef, useS, useTranslate } from '@cloudbeaver/core-blocks';
Expand All @@ -20,6 +20,7 @@
type DataGridRef,
type ICellPosition,
type IDataGridCellRenderer,
type DataGridProps,
} from '@cloudbeaver/plugin-data-grid';
import {
DATA_CONTEXT_DV_PRESENTATION,
Expand Down Expand Up @@ -52,11 +53,6 @@
import { TableColumnHeader } from './TableColumnHeader/TableColumnHeader.js';
import { TableIndexColumnHeader } from './TableColumnHeader/TableIndexColumnHeader.js';

interface IInnerState {
lastCount: number;
lastScrollTop: number;
}

const rowHeight = 24;
const headerHeight = 32;

Expand All @@ -66,7 +62,7 @@
resultIndex,
simple,
className,
dataFormat,

Check warning on line 65 in webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx

View workflow job for this annotation

GitHub Actions / Frontend / Lint 

'dataFormat' is defined but never used
...rest
}) {
const translate = useTranslate();
Expand All @@ -78,13 +74,6 @@
const focusedCell = useRef<ICellPosition | null>(null);
const focusSyncRef = useRef<ICellPosition | null>(null);
const dataGridRef = useRef<DataGridRef>(null);
const innerState = useObjectRef<IInnerState>(
() => ({
lastCount: 0,
lastScrollTop: 0,
}),
false,
);

const selectionAction = (model.source as unknown as ResultSetDataSource).getAction(resultIndex, ResultSetSelectAction);
const viewAction = (model.source as unknown as ResultSetDataSource).getAction(resultIndex, ResultSetViewAction);
Expand Down Expand Up @@ -140,6 +129,7 @@
} else {
key = { column: viewAction.columnKeys[0], row: viewAction.rowKeys[0] };
}
selectionAction.focus(key as IResultSetElementKey);
}

if (!key?.column || !key?.row) {
Expand Down Expand Up @@ -178,92 +168,6 @@
context.set(DATA_CONTEXT_DV_PRESENTATION, { type: DataViewerPresentationType.Data }, id);
});

function handleKeyDown(event: React.KeyboardEvent<HTMLDivElement>) {
gridSelectedCellCopy.onKeydownHandler(event);
const cell = selectionAction.getFocusedElement();
// we can't edit table cells if table doesn't have row identifier, but we can edit new created rows before insert (CB-6063)
const canEdit = model.hasElementIdentifier(resultIndex) || !!(cell && tableData.editor.getElementState(cell) === DatabaseEditChangeType.add);

if (EventContext.has(event, EventStopPropagationFlag) || !canEdit || model.isReadonly(resultIndex)) {
return;
}

const activeElements = selectionAction.getActiveElements();
const activeRows = selectionAction.getActiveRows();

if (!cell) {
return;
}

switch (event.nativeEvent.code) {
case 'Escape': {
tableData.editor.revert(...activeElements);
return;
}
case 'KeyR': {
if (event.altKey) {
if (event.shiftKey) {
tableData.editor.duplicate(...activeRows);
} else {
tableData.editor.add(cell);
}
return;
}
}
}

// const colIdx = tableData.getColumnIndexFromColumnKey(cell.column);
// const rowIdx = tableData.getRowIndexFromKey(cell.row);
// const editingState = tableData.editor.getElementState(cell);

// switch (event.nativeEvent.code) {
// case 'Delete': {
// const filteredRows = activeRows.filter(cell => tableData.editor.getElementState(cell) !== DatabaseEditChangeType.delete);

// if (filteredRows.length > 0) {
// const editor = tableData.editor;
// const firstRow = filteredRows[0]!;
// const editingState = tableData.editor.getElementState(firstRow);

// editor.delete(...filteredRows);

// if (editingState === DatabaseEditChangeType.add) {
// if (rowIdx - 1 > 0) {
// handlers.selectCell({ colIdx, rowIdx: rowIdx - 1 });
// }
// } else {
// if (rowIdx + 1 < tableData.rows.length) {
// handlers.selectCell({ colIdx, rowIdx: rowIdx + 1 });
// }
// }
// }

// return;
// }
// case 'KeyV': {
// if (editingState === DatabaseEditChangeType.delete) {
// return;
// }

// if (event.ctrlKey || event.metaKey) {
// if (!clipboardService.clipboardAvailable || clipboardService.state === 'denied' || tableData.isCellReadonly(cell)) {
// return;
// }

// clipboardService
// .read()
// .then(value => tableData.editor.set(cell, value))
// .catch();
// return;
// }
// }
// }

// if (editingState === DatabaseEditChangeType.delete) {
// return;
// }
}

useLayoutEffect(() => {
function syncEditor(data: IResultSetEditActionData) {
const editor = tableData.editor;
Expand All @@ -277,21 +181,7 @@
const rowIdx = tableData.getRowIndexFromKey(key.row);

if (selectionAction.isFocused(key)) {
const rowTop = rowIdx * rowHeight;
const gridDiv = dataGridDivRef.current;
dataGridRef.current?.scrollToCell({ colIdx });

if (gridDiv) {
if (rowTop < gridDiv.scrollTop - rowHeight + headerHeight) {
gridDiv.scrollTo({
top: rowTop,
});
} else if (rowTop > gridDiv.scrollTop + gridDiv.clientHeight - headerHeight - rowHeight) {
gridDiv.scrollTo({
top: rowTop - gridDiv.clientHeight + headerHeight + rowHeight,
});
}
}
return;
}

Expand All @@ -315,24 +205,8 @@
return () => {
tableData.editor.action.removeHandler(syncEditor);
};
}, [tableData.editor, selectionAction]);

Check warning on line 208 in webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx

View workflow job for this annotation

GitHub Actions / Frontend / Lint 

React Hook useLayoutEffect has missing dependencies: 'handlers' and 'tableData'. Either include them or remove the dependency array

useEffect(() => {
const gridDiv = dataGridDivRef.current;

if (
gridDiv &&
innerState.lastCount > model.source.count &&
model.source.count * rowHeight < gridDiv.scrollTop + gridDiv.clientHeight - headerHeight
) {
gridDiv.scrollTo({
top: model.source.count * rowHeight - gridDiv.clientHeight + headerHeight - 1,
});
}

innerState.lastCount = model.source.count;
}, [model.source.count]);

const handleFocusChange = (position: ICellPosition) => {
focusedCell.current = position;
const columnIndex = position.colIdx;
Expand Down Expand Up @@ -375,7 +249,7 @@
getDataGridApi: () => dataGridRef.current,
focus: restoreFocus,
}),
[model, actions, resultIndex, simple, dataGridRef, gridContainerRef, restoreFocus],

Check warning on line 252 in webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx

View workflow job for this annotation

GitHub Actions / Frontend / Lint 

React Hook useMemo has an unnecessary dependency: 'gridContainerRef'. Either exclude it or remove the dependency array
);

const columnsCount = useCreateGridReactiveValue(
Expand Down Expand Up @@ -517,6 +391,65 @@
return <TextPlaceholder>{translate('data_grid_table_empty_placeholder')}</TextPlaceholder>;
}

const handleCellKeyDown: DataGridProps['onCellKeyDown'] = ({ rowIdx, colIdx }, event) => {
gridSelectedCellCopy.onKeydownHandler(event);
const cell = selectionAction.getFocusedElement();
// we can't edit table cells if table doesn't have row identifier, but we can edit new created rows before insert (CB-6063)
const canEdit = model.hasElementIdentifier(resultIndex) || !!(cell && tableData.editor.getElementState(cell) === DatabaseEditChangeType.add);

if (EventContext.has(event, EventStopPropagationFlag) || !canEdit || model.isReadonly(resultIndex)) {
return;
}

const activeElements = selectionAction.getActiveElements();
const activeRows = selectionAction.getActiveRows();

if (!cell) {
return;
}

switch (event.code) {
case 'Escape': {
tableData.editor.revert(...activeElements);
return;
}
case 'KeyR': {
if (event.altKey) {
if (event.shiftKey) {
tableData.editor.duplicate(...activeRows);
} else {
tableData.editor.add(cell);
}
return;
}
return;
}
case 'Delete': {
event.preventGridDefault();

const filteredRows = activeRows.filter(cell => tableData.editor.getElementState(cell) !== DatabaseEditChangeType.delete);

if (filteredRows.length > 0) {
const editor = tableData.editor;
const firstRow = filteredRows[0]!;
const editingState = tableData.editor.getElementState(firstRow);

editor.delete(...filteredRows);

if (editingState === DatabaseEditChangeType.add) {
if (rowIdx - 1 > 0) {
handlers.selectCell({ colIdx, rowIdx: rowIdx - 1 });
}
} else {
if (rowIdx + 1 < tableData.rows.length) {
handlers.selectCell({ colIdx, rowIdx: rowIdx + 1 });
}
}
}
}
}
};

return (
<DataGridContext.Provider value={gridContext}>
<DataGridSelectionContext.Provider value={gridSelectionContext}>
Expand All @@ -526,7 +459,6 @@
tabIndex={-1}
{...rest}
className={s(styles, { container: true }, className)}
onKeyDown={handleKeyDown}
onMouseDown={onMouseDownHandler}
onMouseMove={onMouseMoveHandler}
>
Expand All @@ -544,6 +476,7 @@
getHeaderResizable={getHeaderResizable}
getRowHeight={() => rowHeight}
getColumnKey={getColumnKey}
onCellKeyDown={handleCellKeyDown}

Check failure on line 479 in webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx

View workflow job for this annotation

GitHub Actions / Frontend / Lint 

Callbacks must be listed after all other props
columnCount={columnsCount}
rowCount={rowsCount}
getRowId={rowIdx => (tableData.rows[rowIdx] ? ResultSetDataKeysUtils.serialize(tableData.rows[rowIdx]) : '')}
Expand Down
Loading