Skip to content

Commit dbe96ec

Browse files
authored
Fix hover tabbing behaviour (microsoft#166657)
Ref microsoft#159088 * Focus back to last focused element when tabbing out of hover * Add focus trap (tab loop) while focused on the hover
1 parent 4bc59a5 commit dbe96ec

File tree

2 files changed

+64
-6
lines changed

2 files changed

+64
-6
lines changed

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ export class HoverService implements IHoverService {
2323
private _currentHoverOptions: IHoverOptions | undefined;
2424
private _currentHover: HoverWidget | undefined;
2525

26+
private _lastFocusedElementBeforeOpen: HTMLElement | undefined;
27+
2628
constructor(
2729
@IInstantiationService private readonly _instantiationService: IInstantiationService,
2830
@IContextViewService private readonly _contextViewService: IContextViewService,
@@ -37,10 +39,16 @@ export class HoverService implements IHoverService {
3739
return undefined;
3840
}
3941
this._currentHoverOptions = options;
42+
if (document.activeElement) {
43+
this._lastFocusedElementBeforeOpen = document.activeElement as HTMLElement;
44+
}
4045

4146
const hoverDisposables = new DisposableStore();
4247
const hover = this._instantiationService.createInstance(HoverWidget, options);
4348
hover.onDispose(() => {
49+
// Required to handle cases such as closing the hover with the escape key
50+
this._lastFocusedElementBeforeOpen?.focus();
51+
4452
// Only clear the current options if it's the current hover, the current options help
4553
// reduce flickering when the same hover is shown multiple times
4654
if (this._currentHoverOptions === options) {
@@ -64,11 +72,11 @@ export class HoverService implements IHoverService {
6472
hoverDisposables.add(addDisposableListener(document, EventType.KEY_DOWN, e => this._keyDown(e, hover)));
6573
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_UP, e => this._keyUp(e, hover)));
6674
hoverDisposables.add(addDisposableListener(document, EventType.KEY_UP, e => this._keyUp(e, hover)));
67-
}
68-
if (options.hideOnKeyDown) {
69-
const focusedElement = document.activeElement;
70-
if (focusedElement) {
71-
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, () => this.hideHover()));
75+
if (options.hideOnKeyDown) {
76+
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, () => {
77+
this.hideHover();
78+
this._lastFocusedElementBeforeOpen?.focus();
79+
}));
7280
}
7381
}
7482

@@ -110,7 +118,10 @@ export class HoverService implements IHoverService {
110118
if (keybinding.getSingleModifierDispatchParts().some(value => !!value) || this._keybindingService.softDispatch(event, event.target)) {
111119
return;
112120
}
113-
this.hideHover();
121+
if (e.key !== 'Tab') {
122+
this.hideHover();
123+
this._lastFocusedElementBeforeOpen?.focus();
124+
}
114125
}
115126

116127
private _keyUp(e: KeyboardEvent, hover: HoverWidget) {
@@ -119,6 +130,7 @@ export class HoverService implements IHoverService {
119130
// Hide if alt is released while the mouse os not over hover/target
120131
if (!hover.isMouseIn) {
121132
this.hideHover();
133+
this._lastFocusedElementBeforeOpen?.focus();
122134
}
123135
}
124136
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export class HoverWidget extends Widget {
5252
private _x: number = 0;
5353
private _y: number = 0;
5454
private _isLocked: boolean = false;
55+
private _addedFocusTrap: boolean = false;
5556

5657
get isDisposed(): boolean { return this._isDisposed; }
5758
get isMouseIn(): boolean { return this._lockMouseTracker.isMouseIn; }
@@ -222,10 +223,55 @@ export class HoverWidget extends Widget {
222223
}
223224
}
224225

226+
private addFocusTrap() {
227+
if (this._addedFocusTrap) {
228+
return;
229+
}
230+
this._addedFocusTrap = true;
231+
232+
// Add a hover tab loop if the hover has at least one element with a valid tabIndex
233+
const firstContainerFocusElement = this._hover.containerDomNode;
234+
const lastContainerFocusElement = this.findLastFocusableChild(this._hover.contentsDomNode);
235+
if (lastContainerFocusElement) {
236+
const beforeContainerFocusElement = dom.prepend(this._hoverContainer, $('div'));
237+
const afterContainerFocusElement = dom.append(this._hoverContainer, $('div'));
238+
beforeContainerFocusElement.tabIndex = 0;
239+
afterContainerFocusElement.tabIndex = 0;
240+
this._register(dom.addDisposableListener(afterContainerFocusElement, 'focus', (e) => {
241+
firstContainerFocusElement.focus();
242+
e.preventDefault();
243+
}));
244+
this._register(dom.addDisposableListener(beforeContainerFocusElement, 'focus', (e) => {
245+
lastContainerFocusElement.focus();
246+
e.preventDefault();
247+
}));
248+
}
249+
}
250+
251+
private findLastFocusableChild(root: Node): HTMLElement | undefined {
252+
if (root.hasChildNodes()) {
253+
for (let i = 0; i < root.childNodes.length; i++) {
254+
const node = root.childNodes.item(root.childNodes.length - i - 1);
255+
if (node.nodeType === node.ELEMENT_NODE) {
256+
const parsedNode = node as HTMLElement;
257+
if (typeof parsedNode.tabIndex === 'number' && parsedNode.tabIndex >= 0) {
258+
return parsedNode;
259+
}
260+
}
261+
const recursivelyFoundElement = this.findLastFocusableChild(node);
262+
if (recursivelyFoundElement) {
263+
return recursivelyFoundElement;
264+
}
265+
}
266+
}
267+
return undefined;
268+
}
269+
225270
public render(container: HTMLElement): void {
226271
container.appendChild(this._hoverContainer);
227272

228273
this.layout();
274+
this.addFocusTrap();
229275
}
230276

231277
public layout() {

0 commit comments

Comments
 (0)