[CB] Render data viewer context menu on the root level by sergeyteleshev · Pull Request #4363 · dbeaver/cloudbeaver

sergeyteleshev · 2026-05-26T09:51:09Z

closes https://github.com/dbeaver/pro/issues/9308

codacy-production · 2026-05-26T09:53:06Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -2 complexity

Metric Results

Complexity -2

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

Copilot

Pull request overview

This PR moves the data viewer context menu rendering to the DataGrid root level so the menu can be positioned globally (rather than per-cell) and re-open reliably on a single right-click (fixing issue #9308).

Changes:

Introduces a grid-level TableMenuContext + DataGridMenuContextProvider to manage active cell + menu coordinates.
Replaces the per-cell embedded context menu with a lightweight trigger button and a single root-level CellMenu.
Adds a new useViewLeave hook to close the menu when the originating cell leaves the viewport, and adds a right-click mousedown propagation blocker to avoid premature dismiss.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Menu/DataGridMenuContextProvider.tsx	Adds grid-level provider to centralize context-menu state and rendering.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Menu/CellMenu.tsx	Implements a single root-level context menu bound to the active cell via context.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Menu/CellMenu.module.css	Hides the menu anchor element since menu is positioned via coordinates.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Formatters/CellFormatter.tsx	Replaces embedded menu with a trigger button that opens the root-level menu.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Formatters/CellFormatter.module.css	Styles the new menu trigger button.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx	Wraps the grid in the new menu provider and blocks right-click dismiss behavior.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/CellRenderer/TableMenuContext.ts	Introduces shared menu state contract for cell/menu integration.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/CellRenderer/CellRenderer.tsx	Routes menu open/close through the shared context and closes on viewport leave.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/CellRenderer/CellContext.ts	Extends `setMenuVisibility` to accept an optional position.
webapp/packages/core-blocks/src/useViewLeave.ts	Adds a new hook to detect when an element leaves the viewport.
webapp/packages/core-blocks/src/index.ts	Re-exports `useViewLeave`.
webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Formatters/Menu/CellMenu.tsx	Removes the old per-cell menu implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

devnaumov · 2026-05-27T09:47:25Z

+      menuPosition,
+      openMenu(cellKey: IGridDataKey, x: number, y: number) {
+        this.activeCellKey = cellKey;
+        this.menuPosition.position = { x, y };


Please use menuPosition.open(event)

devnaumov · 2026-05-27T09:53:03Z

  editionState: DatabaseEditChangeType | null;
-  isMenuVisible: boolean;
-  setMenuVisibility(visible: boolean): void;
+  setMenuVisibility(visible: boolean, position?: { x: number; y: number }): void;


This api should be reworked. There should be openMenu handler that takes event and cell id. This handler should call useContextMenuPosition. Because position is only needed when you open menu and you dont need it when close it

devnaumov · 2026-05-27T10:01:21Z

-    cellContext.setMenuVisibility(visible);
+  function handleMenuTriggerClick(event: React.MouseEvent<HTMLButtonElement>) {
+    const rect = event.currentTarget.getBoundingClientRect();
+    cellContext.setMenuVisibility(true, { x: rect.right - 20, y: rect.bottom });


Cell formatter should not make any calculactions with coordinates. openMenu(event) should

devnaumov · 2026-05-27T10:02:59Z

-      focusable={!cellContext.isMenuVisible}
      className={s(style, { formatter: true })}
      value={cellHolder.value as boolean | null}
+      focusable


We still should have the way to check whether menu is opened, please keep old behaviour

devnaumov · 2026-05-27T10:04:38Z

-            onStateSwitch={handleCellMenuStateSwitch}
-          />
-        </div>
+        <button


Please never user native buttons, use our components from ariakit

devnaumov · 2026-05-27T10:06:55Z

+
+  const { activeCellKey, menuPosition } = tableMenuContext;
+
+  useEffect(() => {


this logic should not be inside cellMenu, it should be on MenuContextProvider level

devnaumov · 2026-05-27T10:09:09Z


    function syncFocus(data: DatabaseDataSelectActionsData<Partial<IGridDataKey>>) {
      if (data.type === 'focus') {
+        const colIdx = data.key?.column ? tableData.getColumnIndexFromColumnKey(data.key.column) : -1;


Please do not modify this method

you sure? we are going to loose this behavior that we can re-open on right click the new cell (it was before so this is gonna be a regression). the thing is this focus sync steals the focus which causes the bug

Screen.Recording.2026-05-27.at.14.59.13.mov

this focus is weird. i dont like its defered behavior with setTimeout
something went wrong when we have added it

wdym by something went wrong? Is something works incorrectly? As I remember we always had some async logic but in the different place

devnaumov · 2026-05-27T10:10:52Z

+import type { IGridDataKey } from '@cloudbeaver/plugin-data-viewer';
+
+export interface ITableMenuContext {
+  activeCellKey: IGridDataKey | null;


Why do we need activeCellKey? we already have cell and position inside cellContext, we should use them?

we moved menu outside of cell context so we need to track active cell id to pass it here:

cloudbeaver/webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/Menu/CellMenu.tsx

Lines 78 to 85 in a56dd8c

useDataContextLink(menu.context, (context, id) => {

context.set(DATA_CONTEXT_DV_DDM, dataGridContext.model, id);

context.set(DATA_CONTEXT_DV_DDM_RESULT_INDEX, dataGridContext.resultIndex, id);

context.set(DATA_CONTEXT_DV_SIMPLE, dataGridContext.simple, id);

context.set(DATA_CONTEXT_DV_ACTIONS, dataGridContext.actions, id);

context.set(DATA_CONTEXT_DV_PRESENTATION_ACTIONS, spreadsheetActions, id);

context.set(DATA_CONTEXT_DV_RESULT_KEY, tableMenuContext.activeCell, id);

});

devnaumov · 2026-05-27T10:10:57Z

+import type { IContextMenuPosition } from '@cloudbeaver/core-blocks';
+import type { IGridDataKey } from '@cloudbeaver/plugin-data-viewer';
+
+export interface ITableMenuContext {


You probably dont need this. You have all 3 fields returned from useContextMenuPosition hook already that lets you open and close menu.

we need to track active cell key and is opened status

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

-
-        this.isMenuVisible = visibility;
+      openMenu(event: React.MouseEvent): void {
+        this.tableMenuContext.openMenu(this.cell, event);


+    context.set(DATA_CONTEXT_DV_SIMPLE, dataGridContext.simple, id);
+    context.set(DATA_CONTEXT_DV_ACTIONS, dataGridContext.actions, id);
+    context.set(DATA_CONTEXT_DV_PRESENTATION_ACTIONS, spreadsheetActions, id);
+    context.set(DATA_CONTEXT_DV_RESULT_KEY, tableMenuContext.activeCell, id);
+  });


Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

SychevAndrey · 2026-05-28T08:25:02Z

+            clientY: event.currentTarget.getBoundingClientRect().top + event.currentTarget.getBoundingClientRect().height,
+          });
+
+          this.cellContext.openMenu(event as unknown as React.MouseEvent);


it's a bit strange that we have event: React.KeyboardEvent as parameter of keyDown, but then we cast it to React.MouseEvent

SychevAndrey · 2026-05-28T08:32:22Z


-  function handleCellMenuStateSwitch(visible: boolean): void {
-    cellContext.setMenuVisibility(visible);
+  function handleMenuTriggerClick(event: React.MouseEvent<HTMLButtonElement>) {


Idk, I expect button is opening when I focus it and press Enter. It's a web standard, I don't think we need to change it.

SychevAndrey · 2026-05-28T08:53:00Z


    function syncFocus(data: DatabaseDataSelectActionsData<Partial<IGridDataKey>>) {
      if (data.type === 'focus') {
+        const colIdx = data.key?.column ? tableData.getColumnIndexFromColumnKey(data.key.column) : -1;


wdym by something went wrong? Is something works incorrectly? As I remember we always had some async logic but in the different place

SychevAndrey · 2026-05-28T08:56:57Z

+  useEffect(() => {
+    const container = dataGridContext.getContainer();
+
+    if (!container) {
+      return;
+    }
+
+    function handleScroll() {
+      tableMenuState.closeMenu();
+    }
+
+    container.addEventListener('scroll', handleScroll, { capture: true });
+    return () => container.removeEventListener('scroll', handleScroll, { capture: true });
+  }, [dataGridContext, tableMenuState]);


do we really need this? Usually I see that a many stays on it's place no matter if it's on the screen or not, or we could do smth like GitHub does:

Screen.Recording.2026-05-28.at.10.54.57.mov

…ia shortcut

dbeaver/pro#9308 moves context menu from the cell level to table level

003171b

sergeyteleshev requested review from SychevAndrey and devnaumov May 26, 2026 09:51

sergeyteleshev self-assigned this May 26, 2026

sergeyteleshev commented May 26, 2026

View reviewed changes

Comment thread webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx Outdated

sergeyteleshev commented May 26, 2026

View reviewed changes

Comment thread webapp/packages/plugin-data-spreadsheet-new/src/DataGrid/DataGridTable.tsx Outdated

sergeyteleshev added 5 commits May 26, 2026 12:07

cleanup

2b53c4b

adds DataGridMenuContextProvider

4eba3a1

uses context menu position from the hook useContextMenuPosition

1415889

closes menu if cell is not visible

aa2aa98

adds useViewLeave hook to close menu if it is out of the view

b5d69a8

SychevAndrey requested a review from Copilot May 26, 2026 14:00

Copilot started reviewing on behalf of SychevAndrey May 26, 2026 14:00 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

sergeyteleshev added 5 commits May 26, 2026 16:42

changes approach for re-opening menus + removes isMenuVisible logic

35ce3f5

removes redundant event prevent default and stopPropogation

0b4517e

fixes focus stealing issue

b468dbf

hides menu on scroll

c403459

cleanup

b58fa3f

devnaumov requested changes May 27, 2026

View reviewed changes

sergeyteleshev added 3 commits May 27, 2026 14:55

pr fixes

3fb9cee

cleanup

1b44760

uses cell key to open table

a56dd8c

sergeyteleshev requested a review from devnaumov May 27, 2026 13:36

SychevAndrey requested a review from Copilot May 27, 2026 14:17

Copilot started reviewing on behalf of SychevAndrey May 27, 2026 14:17 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

pr fixes

c7607c6

sergeyteleshev requested a review from Copilot May 28, 2026 08:44

Copilot started reviewing on behalf of sergeyteleshev May 28, 2026 08:45 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

SychevAndrey reviewed May 28, 2026

View reviewed changes

sergeyteleshev added 6 commits May 28, 2026 14:54

simplifies the menu api

2e1d33d

adds openMenuAt to reduce casting to mouse event while opening menu v…

3074cfa

…ia shortcut

cleanup

6f851ee

removes isMenuOpened guard

216fd18

moves menu to the center of icon when opens from keyboard

6f5abc1

removes onClose handling and focus cell in sync workaround

65c25e9


		const { activeCellKey, menuPosition } = tableMenuContext;

		useEffect(() => {

	useDataContextLink(menu.context, (context, id) => {
	context.set(DATA_CONTEXT_DV_DDM, dataGridContext.model, id);
	context.set(DATA_CONTEXT_DV_DDM_RESULT_INDEX, dataGridContext.resultIndex, id);
	context.set(DATA_CONTEXT_DV_SIMPLE, dataGridContext.simple, id);
	context.set(DATA_CONTEXT_DV_ACTIONS, dataGridContext.actions, id);
	context.set(DATA_CONTEXT_DV_PRESENTATION_ACTIONS, spreadsheetActions, id);
	context.set(DATA_CONTEXT_DV_RESULT_KEY, tableMenuContext.activeCell, id);
	});

Conversation

sergeyteleshev commented May 26, 2026

Uh oh!

codacy-production Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergeyteleshev May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codacy-production Bot commented May 26, 2026 •

edited

Loading

sergeyteleshev May 27, 2026 •

edited

Loading