Fix scrolling behavior in menu while zoomed in

[jeanp413]

Fixes #80047

[sbatten]

@jeanp413 thanks for the PR. and thanks for the pointer about the chrome issue. I notice some of the changes are just rearranging of lines and additionally some changes in the submenuviewitem. Are these changes strictly necessary to address the issue? If not, I would like to keep this PR to the minimum amount of changes.

Additionally, some of the changes are in actionbar, which I am hesitant to take anything that affects more than the menu widget. So, it might be better to pull more of that code up a layer.

[jeanp413]

The { preventScroll: true } is necessary to avoid scrolling up when focusing on the actions-container because of the padding from monaco-action-bar animated vertical

[jeanp413]

This change is necessary. this.scrollableElement must be created before calling this because now it's being used directly in get onScroll()

[sbatten]

@jeanp413 If it isn't necessary to fix the original issue, let's revert it. Testing it, I seet that it works and once we have it to a minimum we can see if we need to worry about the code change in actionbar.ts

[sbatten]

as mentioned, let's focus the changes strictly to what's necessary

[jeanp413]

Reverted some changes. Let me know if that's enough

[sbatten]

@jeanp413 This is looking great, I hope you don't mind but I moved the logic you had in menubar.ts to avoid dismissing the menu on scrollbar clicking into menu.ts, since those clicks should not be propagated.

I also made one additional minor change to ensure the prevent scroll behavior doesn't break other instances of actionbar.

Please take a look and see if you agree with my changes.

[jeanp413]

@sbatten yeah it's great, but I found a minor issue when you click on the scrollbar it doesn't stop tracking it. Seems like it's necessary to let the event propagate because it's used here

vscode/src/vs/base/browser/globalMouseMoveMonitor.ts

Lines 87 to 93 in ede82b1

	for (const element of windowChain) {
	this.hooks.add(dom.addDisposableThrottledListener(element.window.document, 'mousemove',
	(data: R) => this.mouseMoveCallback!(data),
	(lastEvent: R, currentEvent) => this.mouseMoveEventMerger!(lastEvent, currentEvent as MouseEvent)
	));
	this.hooks.add(dom.addDisposableListener(element.window.document, 'mouseup', (e: MouseEvent) => this.stopMonitoring(true)));
	}

[sbatten]

@jeanp413 good catch, updated and merging

[jeanp413]

@sbatten I tested this and it fixes the issue for level 1 menus but it doesn't fix it for submenus with level > 1
I think we need to add the same check here

vscode/src/vs/base/browser/ui/menu/menu.ts

Lines 392 to 395 in 26cb1c8

	this._register(addDisposableListener(this.element, EventType.MOUSE_UP, e => {
	EventHelper.stop(e, true);
	this.onClick(e);
	}));

vscode/src/vs/base/browser/ui/menu/menubar.ts

Lines 371 to 379 in 26cb1c8

	this._register(DOM.addDisposableListener(buttonElement, DOM.EventType.MOUSE_UP, (e) => {
	if (!this.ignoreNextMouseUp) {
	if (this.isFocused) {
	this.onMenuTriggered(MenuBar.OVERFLOW_INDEX, true);
	}
	} else {
	this.ignoreNextMouseUp = false;
	}
	}));