Skip to content

Commit

Permalink
Have hideOnKeyDown be honored everywhere and allow specifying the con…
Browse files Browse the repository at this point in the history
…tainer (microsoft#174303)

* Bug fix: `hideOnKeyDown` was not being honored in all keydown handlers causing the hover to hide when some characters were pressed
* feature: allow specifying the container to render the hover under (will be useful for quick pick later)
  • Loading branch information
TylerLeonhardt authored Feb 14, 2023
1 parent d3caabf commit af4a70d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
5 changes: 5 additions & 0 deletions src/vs/workbench/services/hover/browser/hover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ export interface IHoverOptions {
* - If there are elements in the hover to focus, focus stays inside of the hover when tabbing
*/
trapFocus?: boolean;

/**
* The container to render the hover in.
*/
container?: HTMLElement;
}

export interface IHoverAction {
Expand Down
21 changes: 9 additions & 12 deletions src/vs/workbench/services/hover/browser/hoverService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ export class HoverService implements IHoverService {
hoverDisposables.dispose();
});
const provider = this._contextViewService as IContextViewProvider;
provider.showContextView(new HoverContextViewDelegate(hover, focus));
provider.showContextView(
new HoverContextViewDelegate(hover, focus),
options.container
);
hover.onRequestLayout(() => provider.layout());
if ('targetElements' in options.target) {
for (const element of options.target.targetElements) {
Expand All @@ -70,16 +73,10 @@ export class HoverService implements IHoverService {
}
const focusedElement = <HTMLElement | null>document.activeElement;
if (focusedElement) {
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, e => this._keyDown(e, hover)));
hoverDisposables.add(addDisposableListener(document, EventType.KEY_DOWN, e => this._keyDown(e, hover)));
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, e => this._keyDown(e, hover, !!options.hideOnKeyDown)));
hoverDisposables.add(addDisposableListener(document, EventType.KEY_DOWN, e => this._keyDown(e, hover, !!options.hideOnKeyDown)));
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) {
hoverDisposables.add(addDisposableListener(focusedElement, EventType.KEY_DOWN, () => {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
}));
}
}

if ('IntersectionObserver' in window) {
Expand Down Expand Up @@ -110,7 +107,7 @@ export class HoverService implements IHoverService {
}
}

private _keyDown(e: KeyboardEvent, hover: HoverWidget) {
private _keyDown(e: KeyboardEvent, hover: HoverWidget, hideOnKeyDown: boolean) {
if (e.key === 'Alt') {
hover.isLocked = true;
return;
Expand All @@ -120,7 +117,7 @@ export class HoverService implements IHoverService {
if (keybinding.getSingleModifierDispatchChords().some(value => !!value) || this._keybindingService.softDispatch(event, event.target)) {
return;
}
if (!this._currentHoverOptions?.trapFocus || e.key !== 'Tab') {
if (hideOnKeyDown && (!this._currentHoverOptions?.trapFocus || e.key !== 'Tab')) {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
}
Expand All @@ -129,7 +126,7 @@ export class HoverService implements IHoverService {
private _keyUp(e: KeyboardEvent, hover: HoverWidget) {
if (e.key === 'Alt') {
hover.isLocked = false;
// Hide if alt is released while the mouse os not over hover/target
// Hide if alt is released while the mouse is not over hover/target
if (!hover.isMouseIn) {
this.hideHover();
this._lastFocusedElementBeforeOpen?.focus();
Expand Down

0 comments on commit af4a70d

Please sign in to comment.