microsoft / microsoft/vscode

Touch: dragging inside a menu dismisses the open submenu instead of scrolling

Open
#331,838 1 comment 0 reactions 1 assignee Claimed by @sbatten View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.