Merge pull request microsoft#189186 from microsoft/merogge/trap-focus

trap hover focus for screen reader users, don't update the last focused element if we're reshowing the hover

Author: meganrogge

src/vs/workbench/services/hover/browser/hover.ts

@@ -145,6 +145,7 @@ export interface IHoverOptions {
 	 * Whether to trap focus in the following ways:
 	 * - When the hover closes, focus goes to the element that had focus before the hover opened
 	 * - If there are elements in the hover to focus, focus stays inside of the hover when tabbing
+	 * Note that this is overridden to true when in screen reader optimized mode.
 	 */
 	trapFocus?: boolean;
 

src/vs/workbench/services/hover/browser/hoverService.ts

@@ -17,6 +17,7 @@ import { addDisposableListener, EventType } from 'vs/base/browser/dom';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { ResultKind } from 'vs/platform/keybinding/common/keybindingResolver';
+import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
 
 export class HoverService implements IHoverService {
 	declare readonly _serviceBrand: undefined;
@@ -31,23 +32,27 @@ export class HoverService implements IHoverService {
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@IContextViewService private readonly _contextViewService: IContextViewService,
 		@IContextMenuService contextMenuService: IContextMenuService,
-		@IKeybindingService private readonly _keybindingService: IKeybindingService
+		@IKeybindingService private readonly _keybindingService: IKeybindingService,
+		@IAccessibilityService private readonly _accessibilityService: IAccessibilityService
 	) {
 		contextMenuService.onDidShowContextMenu(() => this.hideHover());
 	}
 
-	showHover(options: Readonly<IHoverOptions>, focus?: boolean): IHoverWidget | undefined {
+	showHover(options: Readonly<IHoverOptions>, focus?: boolean, skipLastFocusedUpdate?: boolean): IHoverWidget | undefined {
 		if (getHoverOptionsIdentity(this._currentHoverOptions) === getHoverOptionsIdentity(options)) {
 			return undefined;
 		}
 		this._currentHoverOptions = options;
 		this._lastHoverOptions = options;
-		if (options.trapFocus && document.activeElement) {
-			this._lastFocusedElementBeforeOpen = document.activeElement as HTMLElement;
-		} else {
-			this._lastFocusedElementBeforeOpen = undefined;
+		const trapFocus = options.trapFocus || this._accessibilityService.isScreenReaderOptimized();
+		// HACK, remove this check when #189076 is fixed
+		if (!skipLastFocusedUpdate) {
+			if (trapFocus && document.activeElement) {
+				this._lastFocusedElementBeforeOpen = document.activeElement as HTMLElement;
+			} else {
+				this._lastFocusedElementBeforeOpen = undefined;
+			}
 		}
-
 		const hoverDisposables = new DisposableStore();
 		const hover = this._instantiationService.createInstance(HoverWidget, options);
 		hover.onDispose(() => {
@@ -114,7 +119,7 @@ export class HoverService implements IHoverService {
 		if (!this._lastHoverOptions) {
 			return;
 		}
-		this.showHover(this._lastHoverOptions, true);
+		this.showHover(this._lastHoverOptions, true, true);
 	}
 
 	private _keyDown(e: KeyboardEvent, hover: HoverWidget, hideOnKeyDown: boolean) {