Touch: dragging inside a menu dismisses the open submenu instead of scrolling
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
Type: Bug
On a touch screen, dragging a finger inside a menu that has an open submenu dismisses the submenu instead of scrolling. Since a drag is the only way a touch user can scroll, any menu item below the fold of a submenu is unreachable by touch: the gesture for scrolling is the gesture for dismissal.
### Steps to reproduce
1. Open VS Code Web (or the desktop workbench with device emulation + touch) on a small viewport, so the menu bar collapses into the hamburger button and menus get a constrained max-height.
2. Tap the hamburger, then tap **File** so its submenu opens. On a phone-sized viewport the File submenu overflows and shows a scrollbar.
3. Drag a finger vertically inside the menu.
**Actual:** the submenu closes at the first drag and focus returns to the parent item. Repeating the drag never scrolls.
**Expected:** the drag scrolls the submenu; the submenu closes only when the parent menu has actually scrolled.
### Root cause
Three pieces interact, all reproducible on current `main` (32b97f54e2ec0341c32d665e1f6f136db29cfe79):
1. `Gesture.dispatchEvent` delivers a gesture to **every** registered target that contains the initial touch point. A submenu is rendered as a DOM descendant of the parent menu's action item (`SubmenuMenuActionViewItem.createSubmenu`), and both the parent menu and each action item register gesture targets, so one finger movement inside the submenu also reaches the parent menu's `Gesture.Change` handler, which calls `setScrollPosition`. Instrumenting the DOM on a device shows ten synthetic `-monaco-gesturechange` events for a single `touchmove`, all with the same `translationY`.
2. `ScrollState`'s change detection compares the **raw, unclamped** scroll position (`Scrollable._setState` uses `ScrollState.equals`, which compares `rawScrollTop`), so a menu that cannot scroll at all still emits `onScroll` events; its clamped position never moves, but the raw one does.
3. The submenu entry dismisses unconditionally on the parent's `onScroll`:
https://github.com/microsoft/vscode/blob/32b97f54e2ec0341c32d665e1f6f136db29cfe79/src/vs/base/browser/ui/menu/menu.ts#L824-L829
So the parent menu "scrolls" without moving, and the submenu closes on it.
Mouse users never notice: a wheel over a menu that cannot scroll changes nothing observable, and a menu that genuinely scrolls should indeed dismiss the submenu, because the submenu is anchored to an item that has moved away.
### Proposed fix
`ScrollEvent` already carries the discriminator: `scrollTopChanged` / `scrollLeftChanged` are computed from the **clamped** positions (`ScrollState.createScrollEvent`), so they are false for these phantom events and true for real movement. Gating the dismissal on them preserves the existing behaviour for menus that really scroll:
```ts
this._register(this.parentData.parent.onScroll(e => {
if (!e.scrollTopChanged && !e.scrollLeftChanged) {
return;
}
if (this.parentData.submenu === this.mysubmenu) {
this.parentData.parent.focus(false);
this.cleanupExistingSubmenu(true);
}
}));
```
I will follow up with a PR.
VS Code version: reproduced on 1.133.0 and the code is unchanged on current main.
Contributor guide
Assessment
This issue has not been assessed yet.