fix: add keyboard support for context menu#1172
fix: add keyboard support for context menu#1172annahaa874 wants to merge 6 commits intoglideapps:mainfrom
Conversation
0ac9112 to
a5c32ac
Compare
| } | ||
| } | ||
| } | ||
| console.log({event, details}, "keys.contextMenu:", keys.contextMenu, overlayOpen ) |
There was a problem hiding this comment.
Please remove this console log.
| buttons: 0 | ||
| }, cancel) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think the else if block should be here. You put it in the rangeSelect === "rect" || rangeSelect === "multi-rect" if clause.
There was a problem hiding this comment.
Since rangeSelect can be "none" | "cell" | "rect" | "multi-cell" | "multi-rect", I placed the new condition above it.
| location: [col, row], | ||
| bounds: event.bounds, | ||
| ctrlKey: false, | ||
| metaKey: false, | ||
| shiftKey: true, |
There was a problem hiding this comment.
All of these properties (location, bounds, ctrlKey, metaKey, and shiftKey) exist in the event object. I recommend using those values instead.
The rowMarkerOffset needs to be considered for the col in location. But I think that is already handled in the location property in the event object.
| localEventX: event.bounds.width / 2, | ||
| localEventY: event.bounds.height / 2, |
There was a problem hiding this comment.
Since this is not important for a keyboard event and there is no mouse pointer position, I suggest setting localEventX and localEventY to NaN.
There was a problem hiding this comment.
Since this is a keyboard event and there is no mouse pointer position, localEventX and localEventY are used to control where the context menu opens. To ensure it opens in the center of the cell, I set them to the cell’s midpoint.
1d7fe8e to
20afebe
Compare
3a86574 to
aa8e457
Compare
3c121fa to
8b65628
Compare
Problem Statement
The data grid previously supported opening context menus only via mouse right-click.
Keyboard users were unable to open context menus using standard shortcuts.
At first glance, handling the contextmenu event appears sufficient. However, this assumption breaks down on macOS, where keyboard shortcuts do not fire a contextmenu event.
Root Cause
The issue has two contributing factors:
Split event handling architecture
Platform limitation on macOS
Relying solely on contextmenu listeners is insufficient
Changes