Tab Navigation for hovercards
When focusing on element with hovercard, user can press space/enter
to open hovercard. To close such hovercard, user needs to press ESC.
When opening hovercard with keyboard, focus is moved to hovercard,
when closing hovercard that was opened with keyboard, focus is moved
to element that is associated with hovercard. Hovercard is not opened
by focusing on elements, since a11y specs define opening hovercard
only on space or enter when on focused element.
Release-Notes: Adding tab navigation for hovercards
Google-Bug-Id: b/216300197
Change-Id: I5b7db203b657b6a8597fc610462375010ffc4033
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
index 352219a..e217a79 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
@@ -33,7 +33,7 @@
});
teardown(() => {
- element.hide(new MouseEvent('click'));
+ element.mouseClickHide(new MouseEvent('click'));
});
test('hovercard is shown', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
index b638bc1..48273ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
@@ -448,7 +448,7 @@
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide(e);
+ this.mouseClickHide(e);
}
private handleClickRemoveFromAttentionSet(e: MouseEvent) {
@@ -479,7 +479,7 @@
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide(e);
+ this.mouseClickHide(e);
}
private reportingDetails() {
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.ts
index 1d67500..7e99bb7 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.ts
@@ -66,12 +66,12 @@
>
</gr-hovercard-account>`
);
- await element.show();
+ await element.show({});
await element.updateComplete;
});
teardown(async () => {
- await element.hide(new MouseEvent('click'));
+ await element.mouseClickHide(new MouseEvent('click'));
await element.updateComplete;
});
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
index 0ae88dd..a17b75f 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
@@ -23,6 +23,8 @@
import {hovercardStyles} from '../../styles/gr-hovercard-styles';
import {sharedStyles} from '../../styles/shared-styles';
import {DependencyRequestEvent} from '../../models/dependency';
+import {addShortcut, Key} from '../../utils/dom-util';
+import {ShortcutController} from '../../elements/lit/shortcut-controller';
interface ReloadEventDetail {
clearPatchset?: boolean;
@@ -36,6 +38,12 @@
*/
const containerId = 'gr-hovercard-container';
+export interface MouseKeyboardOrFocusEvent {
+ keyboardEvent?: KeyboardEvent;
+ mouseEvent?: MouseEvent;
+ focusEvent?: FocusEvent;
+}
+
export function getHovercardContainer(
options: {createIfNotExists: boolean} = {createIfNotExists: false}
): HTMLElement | null {
@@ -127,6 +135,10 @@
isScheduledToHide?: boolean;
+ openedByKeyboard = false;
+
+ private targetCleanups: Array<() => void> = [];
+
static get styles() {
return [sharedStyles, hovercardStyles];
}
@@ -136,9 +148,13 @@
super(...args);
// show the hovercard if mouse moves to hovercard
// this will cancel pending hide as well
- this.addEventListener('mouseenter', this.show);
+ this.addEventListener('mouseenter', () => this.show);
// when leave hovercard, hide it immediately
- this.addEventListener('mouseleave', this.hide);
+ this.addEventListener('mouseleave', () => this.hide);
+ const keyboardController = new ShortcutController(this);
+ keyboardController.addGlobal({key: Key.ESC}, (e: KeyboardEvent) =>
+ this.hide({keyboardEvent: e})
+ );
}
override connectedCallback() {
@@ -165,20 +181,34 @@
// trigger the hovercard, which can annoying for the user, for example
// when added reviewer chips appear in the reply dialog via keyboard
// interaction.
- this._target?.addEventListener('mousemove', this.debounceShow);
- this._target?.addEventListener('focus', this.debounceShow);
- this._target?.addEventListener('mouseleave', this.debounceHide);
- this._target?.addEventListener('blur', this.debounceHide);
- this._target?.addEventListener('click', this.hide);
+ this._target?.addEventListener('mousemove', this.mouseDebounceShow);
+ this._target?.addEventListener('mouseleave', this.mouseDebounceHide);
+ this._target?.addEventListener('blur', this.focusDebounceHide);
+ this._target?.addEventListener('click', this.mouseClickHide);
+ if (this._target) {
+ this.targetCleanups.push(
+ addShortcut(this._target, {key: Key.ENTER}, (e: KeyboardEvent) => {
+ this.show({keyboardEvent: e});
+ })
+ );
+ this.targetCleanups.push(
+ addShortcut(this._target, {key: Key.SPACE}, (e: KeyboardEvent) => {
+ this.show({keyboardEvent: e});
+ })
+ );
+ }
this.addEventListener('request-dependency', this.resolveDep);
}
private removeTargetEventListeners() {
- this._target?.removeEventListener('mousemove', this.debounceShow);
- this._target?.removeEventListener('focus', this.debounceShow);
- this._target?.removeEventListener('mouseleave', this.debounceHide);
- this._target?.removeEventListener('blur', this.debounceHide);
- this._target?.removeEventListener('click', this.hide);
+ this._target?.removeEventListener('mousemove', this.mouseDebounceShow);
+ this._target?.removeEventListener('mouseleave', this.mouseDebounceHide);
+ this._target?.removeEventListener('blur', this.focusDebounceHide);
+ this._target?.removeEventListener('click', this.mouseClickHide);
+ for (const cleanup of this.targetCleanups) {
+ cleanup();
+ }
+ this.targetCleanups = [];
this.removeEventListener('request-dependency', this.resolveDep);
}
@@ -195,7 +225,19 @@
}
}
- readonly debounceHide = () => {
+ readonly mouseDebounceHide = (e: MouseEvent) => {
+ this.debounceHide({mouseEvent: e});
+ };
+
+ readonly mouseDebounceShow = (e: MouseEvent) => {
+ this.debounceShow({mouseEvent: e});
+ };
+
+ readonly focusDebounceHide = (e: FocusEvent) => {
+ this.debounceHide({focusEvent: e});
+ };
+
+ readonly debounceHide = (props: MouseKeyboardOrFocusEvent) => {
this.cancelShowTask();
if (!this._isShowing || this.isScheduledToHide) return;
this.isScheduledToHide = true;
@@ -205,7 +247,7 @@
// This happens when hide immediately through click or mouse leave
// on the hovercard
if (!this.isScheduledToHide) return;
- this.hide();
+ this.hide(props);
},
HIDE_DELAY_MS
);
@@ -277,29 +319,43 @@
);
};
+ mouseClickHide = (e: MouseEvent) => {
+ // If the user is clicking on a link and still hovering over the hovercard
+ // or the user is returning from the hovercard but now hovering over the
+ // target (to stop an annoying flicker effect), just return.
+ if (
+ e &&
+ (e.relatedTarget === this ||
+ (e.target === this && e.relatedTarget === this._target))
+ ) {
+ return;
+ }
+ // We allow hiding hovercards on clicks outside even if the keyboard was
+ // the reason that it was displayed.
+ this.hide({mouseEvent: e});
+ };
+
/**
* Hides/closes the hovercard. This occurs when the user triggers the
* `mouseleave` event on the hovercard's `target` element (as long as the
- * user is not hovering over the hovercard).
+ * user is not hovering over the hovercard). If event not specified
+ * in props, code assumes mouseEvent
*/
- readonly hide = (e?: MouseEvent) => {
+ readonly hide = (props: MouseKeyboardOrFocusEvent) => {
this.cancelHideTask();
this.cancelShowTask();
if (!this._isShowing) {
return;
}
-
- // If the user is now hovering over the hovercard or the user is returning
- // from the hovercard but now hovering over the target (to stop an annoying
- // flicker effect), just return.
- if (e) {
- if (
- e.relatedTarget === this ||
- (e.target === this && e.relatedTarget === this._target)
- ) {
- return;
+ if (!props?.keyboardEvent && this.openedByKeyboard) return;
+ if (this.openedByKeyboard) {
+ if (this._target) {
+ this._target.focus();
}
}
+ // Make sure to reset the keyboard variable so new shows will not
+ // assume keyboard is the reason for opening the hovercard.
+ this.openedByKeyboard = false;
// Mark that the hovercard is not visible and do not allow focusing
this._isShowing = false;
@@ -321,14 +377,14 @@
/**
* Shows/opens the hovercard with a fixed delay.
*/
- readonly debounceShow = () => {
- this.debounceShowBy(SHOW_DELAY_MS);
+ readonly debounceShow = (props: MouseKeyboardOrFocusEvent) => {
+ this.debounceShowBy(SHOW_DELAY_MS, props);
};
/**
* Shows/opens the hovercard with the given delay.
*/
- debounceShowBy(delayMs: number) {
+ debounceShowBy(delayMs: number, props: MouseKeyboardOrFocusEvent) {
this.cancelHideTask();
if (this._isShowing || this.isScheduledToShow) return;
this.isScheduledToShow = true;
@@ -337,7 +393,7 @@
() => {
// This happens when the mouse leaves the target before the delay is over.
if (!this.isScheduledToShow) return;
- this.show();
+ this.show(props);
},
delayMs
);
@@ -352,11 +408,16 @@
/**
* Shows/opens the hovercard. This occurs when the user triggers the
- * `mousenter` event on the hovercard's `target` element.
+ * `mousenter` event on the hovercard's `target` element or when a user
+ * presses enter/space on the hovercard's `target` element. If event not
+ * specified in props, code assumes mouseEvent
*/
- readonly show = async () => {
+ readonly show = async (props: MouseKeyboardOrFocusEvent) => {
this.cancelHideTask();
this.cancelShowTask();
+ // If we are calling show again because of a mouse reason, then keep
+ // the keyboard valuable set.
+ this.openedByKeyboard = this.openedByKeyboard || !!props?.keyboardEvent;
if (this._isShowing || !this.container) {
return;
}
@@ -379,6 +440,9 @@
});
this.updatePosition();
this.classList.remove(HIDE_CLASS);
+ if (props?.keyboardEvent) {
+ this.focus();
+ }
};
updatePosition() {
@@ -490,15 +554,16 @@
_target: HTMLElement | null;
_isShowing: boolean;
dispatchEventThroughTarget(eventName: string, detail?: unknown): void;
- show(): void;
+ show(props: MouseKeyboardOrFocusEvent): void;
// Used for tests
- hide(e: MouseEvent): void;
+ mouseClickHide(e: MouseEvent): void;
+ hide(props: MouseKeyboardOrFocusEvent): void;
container: HTMLElement | null;
hideTask?: DelayedTask;
showTask?: DelayedTask;
position: string;
- debounceShowBy(delayMs: number): void;
+ debounceShowBy(delayMs: number, props: MouseKeyboardOrFocusEvent): void;
updatePosition(): void;
isScheduledToShow?: boolean;
isScheduledToHide?: boolean;
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
index e6b63e6..7bba8c5 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin_test.ts
@@ -19,7 +19,8 @@
import {HovercardMixin} from './hovercard-mixin.js';
import {html, LitElement} from 'lit';
import {customElement} from 'lit/decorators';
-import {MockPromise, mockPromise} from '../../test/test-utils.js';
+import {MockPromise, mockPromise, pressKey} from '../../test/test-utils.js';
+import {findActiveElement, Key} from '../../utils/dom-util.js';
const base = HovercardMixin(LitElement);
@@ -60,7 +61,8 @@
});
teardown(() => {
- element.hide(new MouseEvent('click'));
+ pressKey(element, Key.ESC);
+ element.mouseClickHide(new MouseEvent('click'));
button?.remove();
});
@@ -95,7 +97,7 @@
});
test('hide', () => {
- element.hide(new MouseEvent('click'));
+ element.mouseClickHide(new MouseEvent('click'));
const style = getComputedStyle(element);
assert.isFalse(element._isShowing);
assert.isFalse(element.classList.contains('hovered'));
@@ -104,7 +106,7 @@
});
test('show', async () => {
- await element.show();
+ await element.show({});
await element.updateComplete;
const style = getComputedStyle(element);
assert.isTrue(element._isShowing);
@@ -114,14 +116,14 @@
});
test('debounceShow does not show immediately', async () => {
- element.debounceShowBy(100);
+ element.debounceShowBy(100, {});
setTimeout(() => testPromise.resolve(), 0);
await testPromise;
assert.isFalse(element._isShowing);
});
test('debounceShow shows after delay', async () => {
- element.debounceShowBy(1);
+ element.debounceShowBy(1, {});
setTimeout(() => testPromise.resolve(), 10);
await testPromise;
assert.isTrue(element._isShowing);
@@ -174,4 +176,52 @@
assert.isFalse(element.isScheduledToShow);
assert.isFalse(element._isShowing);
});
+
+ test('do not show on focus', async () => {
+ const button = document.querySelector('button');
+ button?.focus();
+ await element.updateComplete;
+ assert.isNotTrue(element.isScheduledToShow);
+ assert.isFalse(element._isShowing);
+ });
+
+ test('show on pressing enter when focused', async () => {
+ const button = document.querySelector('button')!;
+ button.focus();
+ await element.updateComplete;
+ pressKey(button, Key.ENTER);
+ await element.updateComplete;
+ assert.isTrue(element._isShowing);
+ });
+
+ test('show on pressing space when focused', async () => {
+ const button = document.querySelector('button')!;
+ button.focus();
+ await element.updateComplete;
+ pressKey(button, Key.SPACE);
+ await element.updateComplete;
+ assert.isTrue(element._isShowing);
+ });
+
+ test('when on pressing enter, focus is moved to hovercard', async () => {
+ const button = document.querySelector('button')!;
+ button.focus();
+ await element.updateComplete;
+ await element.show({keyboardEvent: new KeyboardEvent('enter')});
+ await element.updateComplete;
+ assert.isTrue(element._isShowing);
+ const activeElement = findActiveElement(document);
+ assert.isTrue(activeElement === element);
+ });
+
+ test('when on mouseEvent, focus is not moved to hovercard', async () => {
+ const button = document.querySelector('button')!;
+ button.focus();
+ await element.updateComplete;
+ await element.show({mouseEvent: new MouseEvent('enter')});
+ await element.updateComplete;
+ assert.isTrue(element._isShowing);
+ const activeElement = findActiveElement(document);
+ assert.isTrue(activeElement !== element);
+ });
});
