Show extended file status information when comparing patchsets
Screenshots https://imgur.com/a/tklPSAb
Release-Notes: skip
Google-Bug-Id: b/216343564
Change-Id: If9763ca9104fafdc7f172ce0a5af4ad7e7fc5a69
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index d42a40e..cfde4b0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -77,6 +77,7 @@
import {ValueChangedEvent} from '../../../types/events';
import {subscribe} from '../../lit/subscription-controller';
import {when} from 'lit/directives/when';
+import {classMap} from 'lit/directives/class-map';
import {incrementalRepeat} from '../../lit/incremental-repeat';
import {ifDefined} from 'lit/directives/if-defined';
import {KnownExperimentId} from '../../../services/flags/flags';
@@ -218,6 +219,9 @@
files: NormalizedFileInfo[] = [];
// Private but used in tests.
+ @state() filesLeftBase: NormalizedFileInfo[] = [];
+
+ // Private but used in tests.
@state()
loggedIn = false;
@@ -397,6 +401,15 @@
}
.status {
margin-right: var(--spacing-s);
+ display: flex;
+ width: 20px;
+ justify-content: flex-end;
+ }
+ .status.extended {
+ width: 60px;
+ }
+ .status > * {
+ display: block;
}
.path {
cursor: pointer;
@@ -694,6 +707,13 @@
);
subscribe(
this,
+ () => this.getFilesModel().filesLeftBase$,
+ files => {
+ this.filesLeftBase = [...files];
+ }
+ );
+ subscribe(
+ this,
() => this.getBrowserModel().diffViewMode$,
diffView => {
this.diffViewMode = diffView;
@@ -1012,16 +1032,55 @@
private renderFileStatus(file?: NormalizedFileInfo) {
if (!this.flagsService.isEnabled(KnownExperimentId.MORE_FILES_INFO)) return;
- let status: FileInfoStatus | undefined = undefined;
- if (file && !isMagicPath(file?.__path)) {
- status = file.status ?? FileInfoStatus.MODIFIED;
- }
- // TODO: Add support for showing more file info when comparing two patchsets.
- return html`<div class="status" role="gridcell">
- <gr-file-status .status=${status}></gr-file-status>
+ const hasExtendedStatus = this.filesLeftBase.length > 0;
+ const leftStatus = this.renderFileStatusLeft(file?.__path);
+ const rightStatus = this.renderFileStatusRight(file);
+ return html`<div
+ class=${classMap({status: true, extended: hasExtendedStatus})}
+ role="gridcell"
+ >
+ ${leftStatus}${rightStatus}
</div>`;
}
+ private renderFileStatusRight(file?: NormalizedFileInfo) {
+ if (!file) return nothing;
+ if (isMagicPath(file.__path)) return nothing;
+
+ const hasExtendedStatus = this.filesLeftBase.length > 0;
+ const fileWasAlreadyChanged = this.filesLeftBase.some(
+ info => info.__path === file?.__path
+ );
+ const newlyChanged = hasExtendedStatus && !fileWasAlreadyChanged;
+
+ const status = file?.status ?? FileInfoStatus.MODIFIED;
+ const postfix = ` in patchset ${this.patchRange?.patchNum}`;
+
+ return html`<gr-file-status
+ .status=${status}
+ .labelPostfix=${postfix}
+ ?newlyChanged=${newlyChanged}
+ ></gr-file-status>`;
+ }
+
+ private renderFileStatusLeft(path?: string) {
+ if (this.filesLeftBase.length === 0) return nothing;
+ if (isMagicPath(path)) return nothing;
+ const file = this.filesLeftBase.find(info => info.__path === path);
+ if (!file) return nothing;
+
+ const status = file.status ?? FileInfoStatus.MODIFIED;
+ const postfix = ` in patchset ${this.patchRange?.basePatchNum}`;
+
+ return html`
+ <gr-file-status
+ .status=${status}
+ .labelPostfix=${postfix}
+ ></gr-file-status>
+ <iron-icon icon="gr-icons:arrow-right"></iron-icon>
+ `;
+ }
+
private renderFilePath(file: NormalizedFileInfo) {
return html` <span class="path" role="gridcell">
<a class="pathLink" href=${ifDefined(this.computeDiffURL(file.__path))}>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index 8d9afb1..045f9b6 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -15,6 +15,7 @@
stubRestApi,
waitUntil,
pressKey,
+ stubFlags,
} from '../../../test/test-utils';
import {
BasePatchSetNum,
@@ -254,6 +255,22 @@
</div>`);
});
+ test('renders file status column', async () => {
+ stubFlags('isEnabled').returns(true);
+ element.files = createFiles(1, {lines_inserted: 9});
+ element.filesLeftBase = createFiles(1, {lines_inserted: 9});
+ await element.updateComplete;
+ const fileRows = queryAll<HTMLDivElement>(element, '.file-row');
+ const statusCol = queryAndAssert(fileRows?.[0], '.status');
+ expect(statusCol).dom.equal(/* HTML */ `
+ <div class="extended status" role="gridcell">
+ <gr-file-status></gr-file-status>
+ <iron-icon icon="gr-icons:arrow-right"></iron-icon>
+ <gr-file-status></gr-file-status>
+ </div>
+ `);
+ });
+
test('correct number of files are shown', async () => {
element.fileListIncrement = 300;
element.files = createFiles(500);
diff --git a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status.ts b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status.ts
index ab30138..33bbaa0 100644
--- a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status.ts
@@ -10,12 +10,34 @@
import '@polymer/iron-icon/iron-icon';
import {iconStyles} from '../../../styles/gr-icon-styles';
+function statusString(status: FileInfoStatus) {
+ if (!status) return '';
+ switch (status) {
+ case FileInfoStatus.ADDED:
+ return 'Added';
+ case FileInfoStatus.COPIED:
+ return 'Copied';
+ case FileInfoStatus.DELETED:
+ return 'Deleted';
+ case FileInfoStatus.MODIFIED:
+ return 'Modified';
+ case FileInfoStatus.RENAMED:
+ return 'Renamed';
+ case FileInfoStatus.REWRITTEN:
+ return 'Rewritten';
+ case FileInfoStatus.UNMODIFIED:
+ return 'Unchanged';
+ default:
+ assertNever(status, `Unsupported status: ${status}`);
+ }
+}
+
/**
* This is a square colored box with a single letter, which can be used as a
* prefix column before file names.
*
* It can also show an additional "new" icon for indicating that a file was
- * newly added in a patchset.
+ * newly changed in a patchset.
*
* `gr-file-status-chip` was a chip for being shown after a file name. It will
* become obsolete.
@@ -26,18 +48,25 @@
status?: FileInfoStatus;
/**
- * Show an additional "new" icon for indicating that a file was newly added
+ * Show an additional "new" icon for indicating that a file was newly changed
* in a patchset.
*/
@property({type: Boolean})
- new = false;
+ newlyChanged = false;
+
+ /**
+ * What postfix should the tooltip have? For example you can set
+ * ' in ps 5', such that the 'Added' tooltip becomes 'Added in ps 5'.
+ */
+ @property({type: String})
+ labelPostfix = '';
static override get styles() {
return [
iconStyles,
css`
:host {
- display: block;
+ display: inline-block;
}
iron-icon.new {
margin-right: var(--spacing-xs);
@@ -70,51 +99,34 @@
}
override render() {
- return html`${this.renderNew()}${this.renderStatus()}`;
+ return html`${this.renderNewlyChanged()}${this.renderStatus()}`;
}
private renderStatus() {
const classes = ['status', this.status];
- const label = this.computeLabel();
- const text = this.status ?? ' ';
return html`<div
class=${classes.join(' ')}
tabindex="0"
- title=${label}
- aria-label=${label}
+ title=${this.computeLabel()}
+ aria-label=${this.computeLabel()}
>
- ${text}
+ ${this.status ?? ''}
</div>`;
}
- private renderNew() {
- if (!this.new) return;
+ private renderNewlyChanged() {
+ if (!this.newlyChanged) return;
return html`<iron-icon
class="size-16 new"
icon="gr-icons:new"
+ title=${this.computeLabel()}
></iron-icon>`;
}
private computeLabel() {
if (!this.status) return '';
- switch (this.status) {
- case FileInfoStatus.ADDED:
- return 'Added';
- case FileInfoStatus.COPIED:
- return 'Copied';
- case FileInfoStatus.DELETED:
- return 'Deleted';
- case FileInfoStatus.MODIFIED:
- return 'Modified';
- case FileInfoStatus.RENAMED:
- return 'Renamed';
- case FileInfoStatus.REWRITTEN:
- return 'Rewritten';
- case FileInfoStatus.UNMODIFIED:
- return 'Unchanged';
- default:
- assertNever(this.status, `Unsupported status: ${this.status}`);
- }
+ const prefix = this.newlyChanged ? 'Newly ' : '';
+ return `${prefix}${statusString(this.status)}${this.labelPostfix}`;
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status_test.ts b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status_test.ts
index 614b97c8..682f4e9 100644
--- a/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-file-status-chip/gr-file-status_test.ts
@@ -19,7 +19,7 @@
const setStatus = async (status?: FileInfoStatus, newly = false) => {
element.status = status;
- element.new = newly;
+ element.newlyChanged = newly;
await element.updateComplete;
};
@@ -42,8 +42,17 @@
test('newly added', async () => {
await setStatus(FileInfoStatus.ADDED, true);
expect(element).shadowDom.to.equal(/* HTML */ `
- <iron-icon class="new size-16" icon="gr-icons:new"></iron-icon>
- <div class="A status" aria-label="Added" tabindex="0" title="Added">
+ <iron-icon
+ class="new size-16"
+ icon="gr-icons:new"
+ title="Newly Added"
+ ></iron-icon>
+ <div
+ class="A status"
+ aria-label="Newly Added"
+ tabindex="0"
+ title="Newly Added"
+ >
A
</div>
`);
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
index fc38332..3176f61 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
@@ -166,7 +166,7 @@
<!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material+Icons:new_releases -->
<g id="new"><path d="M0 0h24v24H0V0z" fill="none"/><path d="M23 12l-2.44-2.78.34-3.68-3.61-.82-1.89-3.18L12 3 8.6 1.54 6.71 4.72l-3.61.81.34 3.68L1 12l2.44 2.78-.34 3.69 3.61.82 1.89 3.18L12 21l3.4 1.46 1.89-3.18 3.61-.82-.34-3.68L23 12zm-4.51 2.11l.26 2.79-2.74.62-1.43 2.41L12 18.82l-2.58 1.11-1.43-2.41-2.74-.62.26-2.8L3.66 12l1.85-2.12-.26-2.78 2.74-.61 1.43-2.41L12 5.18l2.58-1.11 1.43 2.41 2.74.62-.26 2.79L20.34 12l-1.85 2.11zM11 15h2v2h-2zm0-8h2v6h-2z"/></g>
<!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material+Icons:arrow_right_alt -->
- <g id="arrow_right"><path d="M0 0h24v24H0V0z" fill="none"/><path d="M14 6l-1.41 1.41L16.17 11H4v2h12.17l-3.58 3.59L14 18l6-6z"/></g>
+ <g id="arrow-right"><path d="M0 0h24v24H0V0z" fill="none"/><path d="M14 6l-1.41 1.41L16.17 11H4v2h12.17l-3.58 3.59L14 18l6-6z"/></g>
</defs>
</svg>
</iron-iconset-svg>`;
diff --git a/polygerrit-ui/app/models/change/files-model.ts b/polygerrit-ui/app/models/change/files-model.ts
index 139a5a9..e26167c 100644
--- a/polygerrit-ui/app/models/change/files-model.ts
+++ b/polygerrit-ui/app/models/change/files-model.ts
@@ -121,6 +121,16 @@
([files, commentedPaths]) => addUnmodified(files, commentedPaths)
);
+ public readonly filesLeftBase$ = select(
+ this.state$,
+ state => state.filesLeftBase
+ );
+
+ public readonly filesRightBase$ = select(
+ this.state$,
+ state => state.filesRightBase
+ );
+
private subscriptions: Subscription[] = [];
constructor(
