Skip to content

Commit

Permalink
Fix hover tabbing behaviour (#166657)
Browse files Browse the repository at this point in the history
Ref #159088

* Focus back to last focused element when tabbing out of hover
* Add focus trap (tab loop) while focused on the hover
  • Loading branch information
rzhao271 authored Nov 28, 2022
1 parent 4bc59a5 commit dbe96ec
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 6 deletions.
24 changes: 18 additions & 6 deletions src/vs/workbench/services/hover/browser/hoverService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export class HoverService implements IHoverService {
private _currentHoverOptions: IHoverOptions | undefined;
private _currentHover: HoverWidget | undefined;

private _lastFocusedElementBeforeOpen: HTMLElement | undefined;

constructor(
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IContextViewService private readonly _contextViewService: IContextViewService,
Expand All @@ -37,10 +39,16 @@ export class HoverService implements IHoverService {
return undefined;
}
this._currentHoverOptions = options;
if (document.activeElement) {
this._lastFocusedElementBeforeOpen = document.activeElement as HTMLElement;
}

const hoverDisposables = new DisposableStore();
const hover = this._instantiationService.createInstance(HoverWidget, options);
hover.onDispose(() => {
// Required to handle cases such as closing the hover with the escape key
this._lastFocusedElementBeforeOpen?.focus();

// Only clear the current options if it's the current hover, the current options help
// reduce flickering when the same hover is shown multiple times
if (this._currentHoverOptions === options) {
Expand All @@ -64,11 +72,11 @@ export class HoverService implements IHoverService {
hoverDisposables.add(addDisposableListener(document, EventType.KEY_DOWN, e => this._keyDown(e, hover)));
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_UP, e => this._keyUp(e, hover)));
hoverDisposables.add(addDisposableListener(document, EventType.KEY_UP, e => this._keyUp(e, hover)));
}
if (options.hideOnKeyDown) {
const focusedElement = document.activeElement;
if (focusedElement) {
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, () => this.hideHover()));
if (options.hideOnKeyDown) {
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, () => {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
}));
}
}

Expand Down Expand Up @@ -110,7 +118,10 @@ export class HoverService implements IHoverService {
if (keybinding.getSingleModifierDispatchParts().some(value => !!value) || this._keybindingService.softDispatch(event, event.target)) {
return;
}
this.hideHover();
if (e.key !== 'Tab') {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
}
}

private _keyUp(e: KeyboardEvent, hover: HoverWidget) {
Expand All @@ -119,6 +130,7 @@ export class HoverService implements IHoverService {
// Hide if alt is released while the mouse os not over hover/target
if (!hover.isMouseIn) {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
}
}
}
Expand Down
46 changes: 46 additions & 0 deletions src/vs/workbench/services/hover/browser/hoverWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class HoverWidget extends Widget {
private _x: number = 0;
private _y: number = 0;
private _isLocked: boolean = false;
private _addedFocusTrap: boolean = false;

get isDisposed(): boolean { return this._isDisposed; }
get isMouseIn(): boolean { return this._lockMouseTracker.isMouseIn; }
Expand Down Expand Up @@ -222,10 +223,55 @@ export class HoverWidget extends Widget {
}
}

private addFocusTrap() {
if (this._addedFocusTrap) {
return;
}
this._addedFocusTrap = true;

// Add a hover tab loop if the hover has at least one element with a valid tabIndex
const firstContainerFocusElement = this._hover.containerDomNode;
const lastContainerFocusElement = this.findLastFocusableChild(this._hover.contentsDomNode);
if (lastContainerFocusElement) {
const beforeContainerFocusElement = dom.prepend(this._hoverContainer, $('div'));
const afterContainerFocusElement = dom.append(this._hoverContainer, $('div'));
beforeContainerFocusElement.tabIndex = 0;
afterContainerFocusElement.tabIndex = 0;
this._register(dom.addDisposableListener(afterContainerFocusElement, 'focus', (e) => {
firstContainerFocusElement.focus();
e.preventDefault();
}));
this._register(dom.addDisposableListener(beforeContainerFocusElement, 'focus', (e) => {
lastContainerFocusElement.focus();
e.preventDefault();
}));
}
}

private findLastFocusableChild(root: Node): HTMLElement | undefined {
if (root.hasChildNodes()) {
for (let i = 0; i < root.childNodes.length; i++) {
const node = root.childNodes.item(root.childNodes.length - i - 1);
if (node.nodeType === node.ELEMENT_NODE) {
const parsedNode = node as HTMLElement;
if (typeof parsedNode.tabIndex === 'number' && parsedNode.tabIndex >= 0) {
return parsedNode;
}
}
const recursivelyFoundElement = this.findLastFocusableChild(node);
if (recursivelyFoundElement) {
return recursivelyFoundElement;
}
}
}
return undefined;
}

public render(container: HTMLElement): void {
container.appendChild(this._hoverContainer);

this.layout();
this.addFocusTrap();
}

public layout() {
Expand Down

0 comments on commit dbe96ec

Please sign in to comment.