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.

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

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

SychevAndrey
Copy link
Contributor

@SychevAndrey SychevAndrey commented Mar 25, 2025

closes #5213
CB-6296 Delete row shortcut not working

closes #5178
https://dbeaver.atlassian.net/browse/CB-6229 (see first video)

Value panel fix

Screen.Recording.2025-03-25.at.18.18.02.mov

Delete newly created rows

Screen.Recording.2025-03-25.at.18.17.32.mov

Delete existing rows, cancel on Esc

Screen.Recording.2025-03-25.at.18.17.01.mov

It was noticed that just after the open, onKeyDown event doesn't reach DataGrid, because focus was not in sync with lib.
Also fixes CB-6229 since they have the same root problem with encountered bag
@@ -26,6 +32,7 @@ export interface DataGridProps extends IDataGridCellContext, IDataGridRowContext
onScroll?: (event: React.UIEvent<HTMLDivElement>) => void;
onFocus?: (position: ICellPosition) => void;
onEditorOpen?: (position: ICellPosition) => void;
onCellKeyDown?: (args: CellKeyDownArgs<IInnerRow>, event: CellKeyboardEvent) => void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base grid API should not expose the inner implementation (or third-party types and API's) as much as possible. Please change type to something like: (cell: ICellPosition, event: React.KeyboardEvent<HTMLDivElement>) => void

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

handleFocusChange(focusedCell.current ?? { colIdx: 1, rowIdx: 0 });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it an issue with base grid implementation? We can't use magic numbers here, assuming that there is always at least one column and row (the table can be empty with no rows at all)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach works well with empty tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants