Merge "Migrate apache commons lang v2 usages to v3"
diff --git a/plugins/replication b/plugins/replication
index ba3a9eb..220ce68 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit ba3a9eb92811feef9c2f47287613badee987a9d2
+Subproject commit 220ce6829d4b8382374b08045dc4a1459db8f001
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);
+ });
});
