Display file owners when a owner's status icon is hovered over
When one hoovers over the file owner's status icon the panel is shown
and it contains file owners (up to 5 per file).
TODOS:
* add information if any of them already voted on the change
* add copy button
Bug: Issue 373151160
Change-Id: I8a36e413e35776d70bee5858ea7fd70730e2e4a6
diff --git a/owners/web/gr-owners.ts b/owners/web/gr-owners.ts
index ca761fd..b989eba 100644
--- a/owners/web/gr-owners.ts
+++ b/owners/web/gr-owners.ts
@@ -16,14 +16,16 @@
*/
import {Subscription} from 'rxjs';
-import {css, html, LitElement, nothing, PropertyValues} from 'lit';
+import {css, html, LitElement, nothing, PropertyValues, CSSResult} from 'lit';
import {customElement, property, state} from 'lit/decorators';
import {
+ AccountInfo,
ChangeInfo,
ChangeStatus,
} from '@gerritcodereview/typescript-api/rest-api';
import {
FilesOwners,
+ isOwner,
OWNERS_SUBMIT_REQUIREMENT,
OwnersService,
} from './owners-service';
@@ -45,18 +47,13 @@
[STATUS_CODE.MISSING]: 'schedule',
};
-const STATUS_SUMMARY = {
- [STATUS_CODE.MISSING]: 'Missing',
-};
-
-const STATUS_TOOLTIP = {
- [STATUS_CODE.MISSING]: 'Missing owner approval',
-};
-
const FILE_STATUS = {
[FileStatus.NEEDS_APPROVAL]: STATUS_CODE.MISSING,
};
+const DISPLAY_OWNERS_FOR_FILE_LIMIT = 5;
+const LIMITED_FILES_OWNERS_TOOLTIP = `File owners limited to first ${DISPLAY_OWNERS_FOR_FILE_LIMIT} accounts.`;
+
// Lit mixin definition as described in https://lit.dev/docs/composition/mixins/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type Constructor<T> = new (...args: any[]) => T;
@@ -180,6 +177,7 @@
export class FileOwnersColumnHeader extends common {
static override get styles() {
return [
+ window?.Gerrit?.styles.font as CSSResult,
css`
:host() {
display: block;
@@ -211,8 +209,14 @@
@property({type: String, reflect: true, attribute: 'file-status'})
fileStatus?: string;
+ private owners?: AccountInfo[];
+
+ // taken from Gerrit's common-util.ts
+ private uniqueId = Math.random().toString(36).substring(2);
+
static override get styles() {
return [
+ window?.Gerrit?.styles.font as CSSResult,
css`
:host {
display: flex;
@@ -238,21 +242,110 @@
return nothing;
}
- const info = STATUS_SUMMARY[this.fileStatus];
- const tooltip = STATUS_TOOLTIP[this.fileStatus];
const icon = STATUS_ICON[this.fileStatus];
return html`
- <gr-tooltip-content
- title=${tooltip}
- aria-label=${info}
- aria-description=${info}
- has-tooltip
- >
- <gr-icon class="status" icon=${icon} aria-hidden="true"></gr-icon>
- </gt-tooltip-content>
+ <gr-icon
+ id="${this.pathId()}"
+ class="status"
+ icon=${icon}
+ aria-hidden="true"
+ ></gr-icon>
+ ${this.renderFileOwners()}
`;
}
+ private pathId(): string {
+ return `path-${this.uniqueId}`;
+ }
+
+ private renderFileOwners() {
+ const owners = this.owners ?? [];
+ const splicedOwners = owners.splice(0, DISPLAY_OWNERS_FOR_FILE_LIMIT);
+ const showEllipsis = owners.length > DISPLAY_OWNERS_FOR_FILE_LIMIT;
+ // inlining <style> here is ugly but an alternative would be to copy the `HovercardMixin` from Gerrit and implement hoover from scratch
+ return html`<gr-hovercard for="${this.pathId()}">
+ <style>
+ #file-owners-hoovercard {
+ min-width: 256px;
+ max-width: 256px;
+ margin: -10px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ h3 {
+ font: inherit;
+ }
+ .heading-3 {
+ margin-left: -2px;
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h3);
+ font-weight: var(--font-weight-h3);
+ line-height: var(--line-height-h3);
+ }
+ .row {
+ display: flex;
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ align-items: center;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.sectionIcon gr-icon {
+ position: relative;
+ }
+ div.sectionContent .row {
+ margin-left: 2px;
+ }
+ div.sectionContent .notLast {
+ margin-bottom: 2px;
+ }
+ div.sectionContent .ellipsis {
+ margin-left: 5px;
+ }
+ </style>
+ <div id="file-owners-hoovercard">
+ <div class="section">
+ <div class="sectionIcon">
+ <gr-icon class="status" icon="info" aria-hidden="true"></gr-icon>
+ </div>
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>Needs Owners' Approval</span>
+ </h3>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionContent">
+ ${splicedOwners.map(
+ (owner, idx) => html`
+ <div
+ class="row ${showEllipsis || idx + 1 < splicedOwners.length
+ ? 'notLast'
+ : ''}"
+ >
+ <gr-account-label .account=${owner}></gr-account-label>
+ </div>
+ `
+ )}
+ ${showEllipsis
+ ? html`
+ <gr-tooltip-content
+ title=${LIMITED_FILES_OWNERS_TOOLTIP}
+ aria-label="limited-file-onwers"
+ aria-description=${LIMITED_FILES_OWNERS_TOOLTIP}
+ has-tooltip
+ >
+ <div class="row ellipsis">...</div>
+ </gt-tooltip-content>`
+ : nothing}
+ </div>
+ </div>
+ </div>
+ </gr-hovercard>`;
+ }
+
protected override willUpdate(changedProperties: PropertyValues): void {
super.willUpdate(changedProperties);
this.computeFileState();
@@ -269,10 +362,15 @@
fileOwnership.fileStatus === FileStatus.NOT_OWNED_OR_APPROVED
) {
this.fileStatus = undefined;
+ this.owners = undefined;
return;
}
this.fileStatus = FILE_STATUS[fileOwnership.fileStatus];
+ // TODO for the time being filter out or group owners - to be decided what/how to display them
+ this.owners = (fileOwnership.owners ?? [])
+ .filter(isOwner)
+ .map(o => ({_account_id: o.id} as unknown as AccountInfo));
}
}
@@ -324,9 +422,11 @@
return undefined;
}
- const status =
- allFilesApproved || !(filesOwners.files ?? {})[path]
- ? FileStatus.NOT_OWNED_OR_APPROVED
- : FileStatus.NEEDS_APPROVAL;
- return {fileStatus: status} as unknown as FileOwnership;
+ const fileOwners = (filesOwners.files ?? {})[path];
+ return (allFilesApproved || !fileOwners
+ ? {fileStatus: FileStatus.NOT_OWNED_OR_APPROVED}
+ : {
+ fileStatus: FileStatus.NEEDS_APPROVAL,
+ owners: fileOwners,
+ }) as unknown as FileOwnership;
}
diff --git a/owners/web/gr-owners_test.ts b/owners/web/gr-owners_test.ts
index ff76c9c..0b4a5ec 100644
--- a/owners/web/gr-owners_test.ts
+++ b/owners/web/gr-owners_test.ts
@@ -241,7 +241,10 @@
assert.equal(
deepEqual(
getFileOwnership(path, !allFilesApproved, fileOwnersWithPath),
- {fileStatus: FileStatus.NEEDS_APPROVAL} as FileOwnership
+ {
+ fileStatus: FileStatus.NEEDS_APPROVAL,
+ owners: [{name: 'John', id: 1}],
+ } as FileOwnership
),
true
);
diff --git a/owners/web/owners-model.ts b/owners/web/owners-model.ts
index d1ecaab..5ed7df3 100644
--- a/owners/web/owners-model.ts
+++ b/owners/web/owners-model.ts
@@ -21,7 +21,7 @@
ChangeInfo,
RevisionPatchSetNum,
} from '@gerritcodereview/typescript-api/rest-api';
-import {FilesOwners, OwnersService} from './owners-service';
+import {FileOwner, FilesOwners, OwnersService} from './owners-service';
import {deepEqual} from './utils';
export interface PatchRange {
@@ -56,6 +56,7 @@
*/
export interface FileOwnership {
fileStatus: FileStatus;
+ owners?: FileOwner[];
}
let ownersModel: OwnersModel | undefined;
diff --git a/owners/web/owners-service.ts b/owners/web/owners-service.ts
index 91107bf..479a739 100644
--- a/owners/web/owners-service.ts
+++ b/owners/web/owners-service.ts
@@ -34,6 +34,11 @@
id: number;
}
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function isOwner(o: any): o is Owner {
+ return o && typeof o.id === 'number' && typeof o.name === 'string';
+}
+
export type FileOwner = Owner | GroupOwner;
export interface OwnedFiles {
