diff --git a/src/vs/base/browser/ui/actionbar/actionbar.ts b/src/vs/base/browser/ui/actionbar/actionbar.ts index c8dc421e6d47e..eea4040338184 100644 --- a/src/vs/base/browser/ui/actionbar/actionbar.ts +++ b/src/vs/base/browser/ui/actionbar/actionbar.ts @@ -768,9 +768,9 @@ export class ActionBar extends Disposable implements IActionRunner { this.updateFocus(true); } - protected updateFocus(fromRight?: boolean): void { + protected updateFocus(fromRight?: boolean, preventScroll?: boolean): void { if (typeof this.focusedItem === 'undefined') { - this.actionsList.focus(); + this.actionsList.focus({ preventScroll }); } for (let i = 0; i < this.viewItems.length; i++) { @@ -782,7 +782,7 @@ export class ActionBar extends Disposable implements IActionRunner { if (actionViewItem.isEnabled() && types.isFunction(actionViewItem.focus)) { actionViewItem.focus(fromRight); } else { - this.actionsList.focus(); + this.actionsList.focus({ preventScroll }); } } } else { diff --git a/src/vs/base/browser/ui/menu/menu.ts b/src/vs/base/browser/ui/menu/menu.ts index b6733afe0e5c8..cda0d9db4068d 100644 --- a/src/vs/base/browser/ui/menu/menu.ts +++ b/src/vs/base/browser/ui/menu/menu.ts @@ -16,7 +16,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { Color } from 'vs/base/common/color'; import { DomScrollableElement } from 'vs/base/browser/ui/scrollbar/scrollableElement'; import { ScrollbarVisibility, ScrollEvent } from 'vs/base/common/scrollable'; -import { Event, Emitter } from 'vs/base/common/event'; +import { Event } from 'vs/base/common/event'; import { AnchorAlignment } from 'vs/base/browser/ui/contextview/contextview'; import { isLinux, isMacintosh } from 'vs/base/common/platform'; @@ -66,9 +66,6 @@ export class Menu extends ActionBar { private readonly menuDisposables: DisposableStore; private scrollableElement: DomScrollableElement; private menuElement: HTMLElement; - private scrollTopHold: number | undefined; - - private readonly _onScroll: Emitter; constructor(container: HTMLElement, actions: ReadonlyArray, options: IMenuOptions = {}) { addClass(container, 'monaco-menu-container'); @@ -88,8 +85,6 @@ export class Menu extends ActionBar { this.menuElement = menuElement; - this._onScroll = this._register(new Emitter()); - this.actionsList.setAttribute('role', 'menu'); this.actionsList.tabIndex = 0; @@ -153,17 +148,11 @@ export class Menu extends ActionBar { let relatedTarget = e.relatedTarget as HTMLElement; if (!isAncestor(relatedTarget, this.domNode)) { this.focusedItem = undefined; - this.scrollTopHold = this.menuElement.scrollTop; this.updateFocus(); e.stopPropagation(); } })); - this._register(addDisposableListener(this.domNode, EventType.MOUSE_UP, e => { - // Absorb clicks in menu dead space https://github.com/Microsoft/vscode/issues/63575 - EventHelper.stop(e, true); - })); - this._register(addDisposableListener(this.actionsList, EventType.MOUSE_OVER, e => { let target = e.target as HTMLElement; if (!target || !isAncestor(target, this.actionsList) || target === this.actionsList) { @@ -176,7 +165,6 @@ export class Menu extends ActionBar { if (hasClass(target, 'action-item')) { const lastFocusedItem = this.focusedItem; - this.scrollTopHold = this.menuElement.scrollTop; this.setFocusedItem(target); if (lastFocusedItem !== this.focusedItem) { @@ -191,8 +179,6 @@ export class Menu extends ActionBar { this.mnemonics = new Map>(); - this.push(actions, { icon: true, label: true, isMenu: true }); - // Scroll Logic this.scrollableElement = this._register(new DomScrollableElement(menuElement, { alwaysConsumeMouseWheel: true, @@ -206,19 +192,15 @@ export class Menu extends ActionBar { const scrollElement = this.scrollableElement.getDomNode(); scrollElement.style.position = ''; - menuElement.style.maxHeight = `${Math.max(10, window.innerHeight - container.getBoundingClientRect().top - 30)}px`; + this._register(addDisposableListener(scrollElement, EventType.MOUSE_UP, e => { + // Absorb clicks in menu dead space https://github.com/Microsoft/vscode/issues/63575 + // We do this on the scroll element so the scroll bar doesn't dismiss the menu either + e.preventDefault(); + })); - this.menuDisposables.add(this.scrollableElement.onScroll(() => { - this._onScroll.fire(); - }, this)); + menuElement.style.maxHeight = `${Math.max(10, window.innerHeight - container.getBoundingClientRect().top - 30)}px`; - this._register(addDisposableListener(this.menuElement, EventType.SCROLL, (e: ScrollEvent) => { - if (this.scrollTopHold !== undefined) { - this.menuElement.scrollTop = this.scrollTopHold; - this.scrollTopHold = undefined; - } - this.scrollableElement.scanDomNode(); - })); + this.push(actions, { icon: true, label: true, isMenu: true }); container.appendChild(this.scrollableElement.getDomNode()); this.scrollableElement.scanDomNode(); @@ -254,8 +236,8 @@ export class Menu extends ActionBar { return this.scrollableElement.getDomNode(); } - get onScroll(): Event { - return this._onScroll.event; + get onScroll(): Event { + return this.scrollableElement.onScroll; } get scrollOffset(): number { @@ -295,6 +277,19 @@ export class Menu extends ActionBar { } } + protected updateFocus(fromRight?: boolean): void { + super.updateFocus(fromRight, true); + + if (typeof this.focusedItem !== 'undefined') { + // Workaround for #80047 caused by an issue in chromium + // https://bugs.chromium.org/p/chromium/issues/detail?id=414283 + // When that's fixed, just call this.scrollableElement.scanDomNode() + this.scrollableElement.setScrollPosition({ + scrollTop: Math.round(this.menuElement.scrollTop) + }); + } + } + private doGetActionViewItem(action: IAction, options: IMenuOptions, parentData: ISubMenuData): BaseActionViewItem { if (action instanceof Separator) { return new MenuSeparatorActionViewItem(options.context, action, { icon: true }); diff --git a/src/vs/base/browser/ui/menu/menubar.ts b/src/vs/base/browser/ui/menu/menubar.ts index 4ba4fff01bb1e..8018637cde0ff 100644 --- a/src/vs/base/browser/ui/menu/menubar.ts +++ b/src/vs/base/browser/ui/menu/menubar.ts @@ -274,6 +274,10 @@ export class MenuBar extends Disposable { })); this._register(DOM.addDisposableListener(buttonElement, DOM.EventType.MOUSE_UP, (e) => { + if (e.defaultPrevented) { + return; + } + if (!this.ignoreNextMouseUp) { if (this.isFocused) { this.onMenuTriggered(menuIndex, true);