Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Jul 28, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9123c57
fix #188822
meganrogge Jul 28, 2023
2b61214
Add comment
meganrogge Jul 28, 2023
ca091e2
move comment, add one
meganrogge Jul 28, 2023
ee06aab
Merge branch 'main' into merogge/trap-focus
meganrogge Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/vs/workbench/services/hover/browser/hover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
21 changes: 13 additions & 8 deletions src/vs/workbench/services/hover/browser/hoverService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
meganrogge marked this conversation as resolved.
Show resolved Hide resolved
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(() => {
Expand Down Expand Up @@ -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) {
Expand Down