Merge changes I890b6c61,I8d117211
* changes:
DeleteZombieCommentsRefs: allow the program to run in logging mode
DeleteZombieCommentsRefs: add a logging statement of the deleted drafts
diff --git a/java/com/google/gerrit/asciidoctor/DocIndexer.java b/java/com/google/gerrit/asciidoctor/DocIndexer.java
index 513bdd7..70857c0 100644
--- a/java/com/google/gerrit/asciidoctor/DocIndexer.java
+++ b/java/com/google/gerrit/asciidoctor/DocIndexer.java
@@ -42,8 +42,8 @@
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexWriterConfig;
import org.apache.lucene.index.IndexWriterConfig.OpenMode;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.store.RAMDirectory;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
@@ -92,9 +92,9 @@
}
}
- private RAMDirectory index()
+ private ByteBuffersDirectory index()
throws IOException, UnsupportedEncodingException, FileNotFoundException {
- RAMDirectory directory = new RAMDirectory();
+ ByteBuffersDirectory directory = new ByteBuffersDirectory();
IndexWriterConfig config = new IndexWriterConfig(new StandardAnalyzer(CharArraySet.EMPTY_SET));
config.setOpenMode(OpenMode.CREATE);
config.setCommitOnClose(true);
@@ -132,7 +132,7 @@
return directory;
}
- private byte[] zip(RAMDirectory dir) throws IOException {
+ private byte[] zip(ByteBuffersDirectory dir) throws IOException {
ByteArrayOutputStream buf = new ByteArrayOutputStream();
try (ZipOutputStream zip = new ZipOutputStream(buf)) {
for (String name : dir.listAll()) {
diff --git a/java/com/google/gerrit/lucene/LuceneAccountIndex.java b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
index fe3fc15..7ed28f1 100644
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -50,9 +50,9 @@
import org.apache.lucene.search.SearcherFactory;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
-import org.apache.lucene.store.RAMDirectory;
import org.apache.lucene.util.BytesRef;
import org.eclipse.jgit.lib.Config;
@@ -84,7 +84,7 @@
private static Directory dir(Schema<AccountState> schema, Config cfg, SitePaths sitePaths)
throws IOException {
if (LuceneIndexModule.isInMemoryTest(cfg)) {
- return new RAMDirectory();
+ return new ByteBuffersDirectory();
}
Path indexDir = LuceneVersionManager.getDir(sitePaths, ACCOUNTS, schema);
return FSDirectory.open(indexDir);
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index cf176ee..ff2bd53 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -83,7 +83,7 @@
import org.apache.lucene.search.SortField;
import org.apache.lucene.search.TopDocs;
import org.apache.lucene.search.TopFieldDocs;
-import org.apache.lucene.store.RAMDirectory;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.util.BytesRef;
import org.eclipse.jgit.lib.Config;
@@ -170,7 +170,7 @@
new ChangeSubIndex(
schema,
sitePaths,
- new RAMDirectory(),
+ new ByteBuffersDirectory(),
"ramOpen",
skipFields,
openConfig,
@@ -180,7 +180,7 @@
new ChangeSubIndex(
schema,
sitePaths,
- new RAMDirectory(),
+ new ByteBuffersDirectory(),
"ramClosed",
skipFields,
closedConfig,
diff --git a/java/com/google/gerrit/lucene/LuceneGroupIndex.java b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
index f7a2248..02d5a8b 100644
--- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
@@ -46,9 +46,9 @@
import org.apache.lucene.search.SearcherFactory;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
-import org.apache.lucene.store.RAMDirectory;
import org.apache.lucene.util.BytesRef;
import org.eclipse.jgit.lib.Config;
@@ -74,7 +74,7 @@
private static Directory dir(Schema<?> schema, Config cfg, SitePaths sitePaths)
throws IOException {
if (LuceneIndexModule.isInMemoryTest(cfg)) {
- return new RAMDirectory();
+ return new ByteBuffersDirectory();
}
Path indexDir = LuceneVersionManager.getDir(sitePaths, GROUPS, schema);
return FSDirectory.open(indexDir);
diff --git a/java/com/google/gerrit/lucene/LuceneProjectIndex.java b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
index 11707be..081e259 100644
--- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
@@ -47,9 +47,9 @@
import org.apache.lucene.search.SearcherFactory;
import org.apache.lucene.search.Sort;
import org.apache.lucene.search.SortField;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
-import org.apache.lucene.store.RAMDirectory;
import org.apache.lucene.util.BytesRef;
import org.eclipse.jgit.lib.Config;
@@ -74,7 +74,7 @@
private static Directory dir(Schema<ProjectData> schema, Config cfg, SitePaths sitePaths)
throws IOException {
if (LuceneIndexModule.isInMemoryTest(cfg)) {
- return new RAMDirectory();
+ return new ByteBuffersDirectory();
}
Path indexDir = LuceneVersionManager.getDir(sitePaths, PROJECTS, schema);
return FSDirectory.open(indexDir);
diff --git a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
index 59ae6f8..7d3ddf1 100644
--- a/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
+++ b/java/com/google/gerrit/server/documentation/QueryDocumentationExecutor.java
@@ -33,9 +33,9 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreDoc;
import org.apache.lucene.search.TopDocs;
+import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IndexOutput;
-import org.apache.lucene.store.RAMDirectory;
@Singleton
public class QueryDocumentationExecutor {
@@ -100,7 +100,7 @@
}
protected Directory readIndexDirectory() throws IOException {
- Directory dir = new RAMDirectory();
+ Directory dir = new ByteBuffersDirectory();
byte[] buffer = new byte[4096];
InputStream index = getClass().getResourceAsStream(Constants.INDEX_ZIP);
if (index == null) {
diff --git a/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
index a176bc4..bbd617c 100644
--- a/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
+++ b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.common.SubmitRequirementInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.CurrentUser;
@@ -107,14 +106,11 @@
return Response.created(SubmitRequirementJson.format(submitRequirement));
} catch (ConfigInvalidException e) {
throw new IOException("Failed to read project config", e);
- } catch (ResourceConflictException e) {
- throw new BadRequestException("Failed to create submit requirement", e);
}
}
public SubmitRequirement createSubmitRequirement(
- ProjectConfig config, String name, SubmitRequirementInput input)
- throws BadRequestException, ResourceConflictException {
+ ProjectConfig config, String name, SubmitRequirementInput input) throws BadRequestException {
validateSRName(name);
if (Strings.isNullOrEmpty(input.submittabilityExpression)) {
throw new BadRequestException("submittability_expression is required");
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
index 7bdb741..0bbddc4 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -38,6 +38,7 @@
import {allSettled} from '../../../utils/async-util';
import {pluralize} from '../../../utils/string-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
@customElement('gr-change-list-bulk-vote-flow')
export class GrChangeListBulkVoteFlow extends LitElement {
@@ -53,6 +54,8 @@
@query('#actionOverlay') actionOverlay!: GrOverlay;
+ @query('gr-dialog') dialog?: GrDialog;
+
@state() account?: AccountInfo;
static override get styles() {
@@ -153,10 +156,7 @@
permittedLabels
).filter(label => !triggerLabels.some(l => l.name === label.name));
return html`
- <gr-button
- id="voteFlowButton"
- flatten
- @click=${() => this.actionOverlay.open()}
+ <gr-button id="voteFlowButton" flatten @click=${this.openOverlay}
>Vote</gr-button
>
<gr-overlay id="actionOverlay" with-backdrop="">
@@ -219,6 +219,14 @@
}
}
+ private async openOverlay() {
+ await this.actionOverlay.open();
+ this.actionOverlay.setFocusStops({
+ start: queryAndAssert(this.dialog, 'header'),
+ end: queryAndAssert(this.dialog, 'footer'),
+ });
+ }
+
private renderErrors() {
if (getOverallStatus(this.progressByChange) !== ProgressStatus.FAILED) {
return nothing;
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary.ts
index 6827870..1bfab48 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary.ts
@@ -105,7 +105,7 @@
return this.renderState(
iconForStatus(SubmitRequirementStatus.UNSATISFIED),
- this.renderSummary(numUnsatisfied, numRequirements)
+ this.renderSummary(numUnsatisfied)
);
}
@@ -118,11 +118,8 @@
>`;
}
- renderSummary(numUnsatisfied: number, numRequirements: number) {
- return html`<span
- ><span class="unsatisfied">${numUnsatisfied}</span
- ><span class="total">(of ${numRequirements})</span></span
- >`;
+ renderSummary(numUnsatisfied: number) {
+ return html`<span class="unsatisfied">${numUnsatisfied} missing</span>`;
}
renderCommentIcon() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary_test.ts
index e5e27ce..bf09400 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-column-requirements-summary/gr-change-list-column-requirements-summary_test.ts
@@ -65,10 +65,7 @@
<gr-submit-requirement-dashboard-hovercard>
</gr-submit-requirement-dashboard-hovercard>
<iron-icon class="block" icon="gr-icons:block" role="img"></iron-icon>
- <span>
- <span class="unsatisfied">1</span>
- <span class="total">(of 1)</span>
- </span>
+ <span class="unsatisfied">1 missing</span>
</span>`);
});
@@ -89,10 +86,7 @@
<gr-submit-requirement-dashboard-hovercard>
</gr-submit-requirement-dashboard-hovercard>
<iron-icon class="block" icon="gr-icons:block" role="img"></iron-icon>
- <span>
- <span class="unsatisfied">1</span>
- <span class="total">(of 1)</span>
- </span>
+ <span class="unsatisfied">1 missing</span>
</span>
<iron-icon
class="commentIcon"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
index ea51514..230244e3 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-reviewer-flow/gr-change-list-reviewer-flow.ts
@@ -37,10 +37,11 @@
} from '../../shared/gr-account-list/gr-account-list';
import '@polymer/iron-icon/iron-icon';
import {getReplyByReason} from '../../../utils/attention-set-util';
-import {intersection} from '../../../utils/common-util';
+import {intersection, queryAndAssert} from '../../../utils/common-util';
import {accountOrGroupKey} from '../../../utils/account-util';
import {ValueChangedEvent} from '../../../types/events';
import {fireAlert, fireReload} from '../../../utils/event-util';
+import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
@customElement('gr-change-list-reviewer-flow')
export class GrChangeListReviewerFlow extends LitElement {
@@ -88,6 +89,8 @@
@query('gr-overlay#confirm-cc') private ccConfirmOverlay?: GrOverlay;
+ @query('gr-dialog') dialog?: GrDialog;
+
private readonly reportingService = getAppContext().reportingService;
private getBulkActionsModel = resolve(this, bulkActionsModelToken);
@@ -360,10 +363,15 @@
.map(reviewer => getDisplayName(this.serverConfig, reviewer));
}
- private openOverlay() {
+ private async openOverlay() {
this.resetFlow();
this.isOverlayOpen = true;
- this.overlay?.open();
+ // Must await the overlay opening because the dialog is lazily rendered.
+ await this.overlay?.open();
+ this.overlay?.setFocusStops({
+ start: queryAndAssert(this.dialog, 'header'),
+ end: queryAndAssert(this.dialog, 'footer'),
+ });
}
private closeOverlay() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index c33a50e..35b1751 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -29,8 +29,7 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css, nothing} from 'lit';
import {customElement, property, state} from 'lit/decorators';
-import {ShortcutController} from '../../lit/shortcut-controller';
-import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
+import {Shortcut, ShortcutController} from '../../lit/shortcut-controller';
import {queryAll} from '../../../utils/common-util';
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {Execution} from '../../../constants/reporting';
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index ac487c3..8149f5a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -34,10 +34,6 @@
import '../../checks/gr-checks-tab';
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {
- Shortcut,
- ShortcutSection,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
import {querySelectorAll, windowLocationReload} from '../../../utils/dom-util';
@@ -160,7 +156,11 @@
getRemovedByReason,
hasAttention,
} from '../../../utils/attention-set-util';
-import {shortcutsServiceToken} from '../../../services/shortcuts/shortcuts-service';
+import {
+ Shortcut,
+ ShortcutSection,
+ shortcutsServiceToken,
+} from '../../../services/shortcuts/shortcuts-service';
import {LoadingStatus} from '../../../models/change/change-model';
import {commentsModelToken} from '../../../models/comments/comments-model';
import {resolve} from '../../../models/dependency';
@@ -2087,10 +2087,6 @@
if (this.params.basePatchNum === undefined)
this.params.basePatchNum = PARENT;
- if (this.params.patchNum === undefined) {
- this.params.patchNum = computeLatestPatchNum(this.allPatchSets);
- }
-
const patchChanged = this.hasPatchRangeChanged(this.params);
let patchNumChanged = this.hasPatchNumChanged(this.params);
@@ -2121,7 +2117,7 @@
if (patchChanged) {
// We need to collapse all diffs when params change so that a non
// existing diff is not requested. See Issue 125270 for more details.
- this.fileList?.resetNumFilesShown();
+ this.fileList?.resetFileState();
this.fileList?.collapseAllDiffs();
this.reloadPatchNumDependentResources(patchNumChanged).then(() => {
this.sendShowChangeEvent();
@@ -2146,7 +2142,7 @@
this.updateComplete.then(() => {
assertIsDefined(this.fileList);
this.fileList?.collapseAllDiffs();
- this.fileList?.resetNumFilesShown();
+ this.fileList?.resetFileState();
});
// If the change was loaded before, then we are firing a 'reload' event
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 32feb6a..712e0c8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1354,6 +1354,7 @@
assertIsDefined(element.fileList);
assert.equal(element.fileList.numFilesShown, DEFAULT_NUM_FILES_SHOWN);
element.fileList.numFilesShown = 150;
+ element.fileList.selectedIndex = 15;
await element.updateComplete;
element.changeNum = 2 as NumericChangeId;
@@ -1363,6 +1364,7 @@
};
await element.updateComplete;
assert.equal(element.fileList.numFilesShown, DEFAULT_NUM_FILES_SHOWN);
+ assert.equal(element.fileList.selectedIndex, 0);
});
test('don’t reload entire page when patchRange changes', async () => {
@@ -1386,6 +1388,7 @@
assert.isTrue(reloadStub.calledOnce);
element.initialLoadComplete = true;
+ element.fileList.selectedIndex = 15;
element.change = {
...createChangeViewChange(),
revisions: {
@@ -1399,6 +1402,7 @@
element.params = {...value};
await element.updateComplete;
await flush();
+ assert.equal(element.fileList.selectedIndex, 0);
assert.isFalse(reloadStub.calledTwice);
assert.isTrue(reloadPatchDependentStub.calledOnce);
assert.isTrue(collapseStub.calledTwice);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 762a90a..209c3b7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -28,15 +28,15 @@
import {GrDiffModeSelector} from '../../../embed/diff/gr-diff-mode-selector/gr-diff-mode-selector';
import {GrButton} from '../../shared/gr-button/gr-button';
import {fireEvent} from '../../../utils/event-util';
-import {
- Shortcut,
- ShortcutSection,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {css, html, LitElement} from 'lit';
import {sharedStyles} from '../../../styles/shared-styles';
import {when} from 'lit/directives/when';
import {ifDefined} from 'lit/directives/if-defined';
-import {shortcutsServiceToken} from '../../../services/shortcuts/shortcuts-service';
+import {
+ Shortcut,
+ ShortcutSection,
+ shortcutsServiceToken,
+} from '../../../services/shortcuts/shortcuts-service';
import {resolve} from '../../../models/dependency';
import {getAppContext} from '../../../services/app-context';
import {subscribe} from '../../lit/subscription-controller';
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 4179ad1..88255b7 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
@@ -188,8 +188,7 @@
@property({type: Object})
changeComments?: ChangeComments;
- @property({type: Number, attribute: 'selected-index'})
- selectedIndex = -1;
+ @state() selectedIndex = 0;
@property({type: Object})
change?: ParsedChangeInfo;
@@ -1535,8 +1534,10 @@
);
}
- resetNumFilesShown() {
+ resetFileState() {
this.numFilesShown = DEFAULT_NUM_FILES_SHOWN;
+ this.selectedIndex = 0;
+ this.fileCursor.setCursorAtIndex(this.selectedIndex, true);
}
openDiffPrefs() {
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 d96ad74..aab1afc 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
@@ -986,7 +986,6 @@
test('r key sets reviewed flag', async () => {
await element.updateComplete;
- element.handleCursorNext(new KeyboardEvent('keydown'));
pressKey(element, 'r');
await element.updateComplete;
@@ -998,7 +997,6 @@
element.reviewed = ['/COMMIT_MSG'];
await element.updateComplete;
- element.handleCursorNext(new KeyboardEvent('keydown'));
pressKey(element, 'r');
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index 14a87ec..32bd07a 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -8,10 +8,6 @@
import '../../shared/gr-icons/gr-icons';
import '../gr-message/gr-message';
import '../../../styles/gr-paper-styles';
-import {
- Shortcut,
- ShortcutSection,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {parseDate} from '../../../utils/date-util';
import {MessageTag} from '../../../constants/constants';
import {getAppContext} from '../../../services/app-context';
@@ -43,7 +39,11 @@
import {paperStyles} from '../../../styles/gr-paper-styles';
import {when} from 'lit/directives/when';
import {ifDefined} from 'lit/directives/if-defined';
-import {shortcutsServiceToken} from '../../../services/shortcuts/shortcuts-service';
+import {
+ Shortcut,
+ ShortcutSection,
+ shortcutsServiceToken,
+} from '../../../services/shortcuts/shortcuts-service';
/**
* The content of the enum is also used in the UI for the button text.
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index f6ff2a8..8cd93c5 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -684,7 +684,7 @@
}
override disconnectedCallback() {
- this.storeTask?.cancel();
+ this.storeTask?.flush();
for (const cleanup of this.cleanups) cleanup();
this.cleanups = [];
super.disconnectedCallback();
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index dbd428a..cc8d6c2 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -19,7 +19,7 @@
extractAssociatedLabels,
getApprovalInfo,
hasVotes,
- iconForStatus,
+ iconForRequirement,
} from '../../../utils/label-util';
import {ParsedChangeInfo} from '../../../types/types';
import {css, html, LitElement} from 'lit';
@@ -129,12 +129,9 @@
override render() {
if (!this.requirement) return;
- const icon = iconForStatus(this.requirement.status);
return html` <div id="container" role="tooltip" tabindex="-1">
<div class="section">
- <div class="sectionIcon">
- <iron-icon class=${icon} icon="gr-icons:${icon}"></iron-icon>
- </div>
+ <div class="sectionIcon">${this.renderStatus(this.requirement)}</div>
<div class="sectionContent">
<h3 class="name heading-3">
<span>${this.requirement.name}</span>
@@ -158,6 +155,16 @@
</div>`;
}
+ private renderStatus(requirement: SubmitRequirementResultInfo) {
+ const icon = iconForRequirement(requirement);
+ return html`<iron-icon
+ class=${icon}
+ icon="gr-icons:${icon}"
+ role="img"
+ aria-label=${requirement.status.toLowerCase()}
+ ></iron-icon>`;
+ }
+
private renderDescription() {
let description = this.requirement?.description;
if (this.requirement?.status === SubmitRequirementStatus.ERROR) {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
index 2b2d68d..7670ee80 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_test.ts
@@ -41,6 +41,8 @@
<div class="section">
<div class="sectionIcon">
<iron-icon
+ aria-label="satisfied"
+ role="img"
class="check-circle-filled"
icon="gr-icons:check-circle-filled"
>
@@ -88,6 +90,8 @@
<div class="section">
<div class="sectionIcon">
<iron-icon
+ aria-label="satisfied"
+ role="img"
class="check-circle-filled"
icon="gr-icons:check-circle-filled"
>
@@ -167,6 +171,8 @@
<div class="section">
<div class="sectionIcon">
<iron-icon
+ aria-label="satisfied"
+ role="img"
class="check-circle-filled"
icon="gr-icons:check-circle-filled"
>
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index af08d68..0941096 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -27,7 +27,7 @@
getTriggerVotes,
hasNeutralStatus,
hasVotes,
- iconForStatus,
+ iconForRequirement,
orderSubmitRequirements,
} from '../../../utils/label-util';
import {fontStyles} from '../../../styles/gr-font-styles';
@@ -191,7 +191,7 @@
index: number
) {
const row = html`
- <td>${this.renderStatus(requirement.status)}</td>
+ <td>${this.renderStatus(requirement)}</td>
<td class="name">
<gr-limited-text
class="name"
@@ -242,13 +242,13 @@
</gr-endpoint-decorator>`;
}
- renderStatus(status: SubmitRequirementStatus) {
- const icon = iconForStatus(status);
+ private renderStatus(requirement: SubmitRequirementResultInfo) {
+ const icon = iconForRequirement(requirement);
return html`<iron-icon
class=${icon}
icon="gr-icons:${icon}"
role="img"
- aria-label=${status.toLowerCase()}
+ aria-label=${requirement.status.toLowerCase()}
></iron-icon>`;
}
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
index 73e5da3..de1fd3e 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.ts
@@ -12,8 +12,6 @@
import {
ShortcutSection,
SectionView,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
-import {
shortcutsServiceToken,
ShortcutViewListener,
} from '../../../services/shortcuts/shortcuts-service';
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
index 4099979..e04c48c 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog_test.ts
@@ -9,7 +9,7 @@
import {
SectionView,
ShortcutSection,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
+} from '../../../services/shortcuts/shortcuts-service';
const basicFixture = fixtureFromElement('gr-keyboard-shortcuts-dialog');
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index 5de2190..e614ebe 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -10,9 +10,7 @@
AutocompleteSuggestion,
GrAutocomplete,
} from '../../shared/gr-autocomplete/gr-autocomplete';
-import {getDocsBaseUrl} from '../../../utils/url-util';
import {MergeabilityComputationBehavior} from '../../../constants/constants';
-import {getAppContext} from '../../../services/app-context';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
import {
@@ -21,10 +19,12 @@
state,
query as queryDec,
} from 'lit/decorators';
-import {ShortcutController} from '../../lit/shortcut-controller';
+import {Shortcut, ShortcutController} from '../../lit/shortcut-controller';
import {query as queryUtil} from '../../../utils/common-util';
import {assertIsDefined} from '../../../utils/common-util';
-import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
+import {configModelToken} from '../../../models/config/config-model';
+import {resolve} from '../../../models/dependency';
+import {subscribe} from '../../lit/subscription-controller';
// Possible static search options for auto complete, without negations.
const SEARCH_OPERATORS: ReadonlyArray<string> = [
@@ -153,9 +153,12 @@
@property({type: Object})
accountSuggestions: SuggestionProvider = () => Promise.resolve([]);
- @property({type: Object})
+ @state()
serverConfig?: ServerInfo;
+ @state()
+ mergeabilityComputationBehavior?: MergeabilityComputationBehavior;
+
@property({type: String})
label = '';
@@ -163,22 +166,32 @@
@state() inputVal = '';
// private but used in test
- @state() docBaseUrl: string | null = null;
+ @state() docsBaseUrl: string | null = null;
@state() private query: AutocompleteQuery;
@state() private threshold = 1;
- private searchOperators = new Set(SEARCH_OPERATORS_WITH_NEGATIONS_SET);
-
- private readonly restApiService = getAppContext().restApiService;
-
private readonly shortcuts = new ShortcutController(this);
+ private readonly getConfigModel = resolve(this, configModelToken);
+
constructor() {
super();
this.query = (input: string) => this.getSearchSuggestions(input);
this.shortcuts.addAbstract(Shortcut.SEARCH, () => this.handleSearch());
+ subscribe(
+ this,
+ () => this.getConfigModel().mergeabilityComputationBehavior$,
+ mergeabilityComputationBehavior => {
+ this.mergeabilityComputationBehavior = mergeabilityComputationBehavior;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().docsBaseUrl$,
+ docsBaseUrl => (this.docsBaseUrl = docsBaseUrl)
+ );
}
static override get styles() {
@@ -237,47 +250,34 @@
}
override willUpdate(changedProperties: PropertyValues) {
- if (changedProperties.has('serverConfig')) {
- this.serverConfigChanged();
- }
-
if (changedProperties.has('value')) {
this.valueChanged();
}
}
- private serverConfigChanged() {
- const mergeability =
- this.serverConfig?.change?.mergeability_computation_behavior;
- if (
- mergeability ===
- MergeabilityComputationBehavior.API_REF_UPDATED_AND_CHANGE_REINDEX ||
- mergeability ===
- MergeabilityComputationBehavior.REF_UPDATED_AND_CHANGE_REINDEX
- ) {
- // add 'is:mergeable' to searchOperators
- this.searchOperators.add('is:mergeable');
- this.searchOperators.add('-is:mergeable');
- } else {
- this.searchOperators.delete('is:mergeable');
- this.searchOperators.delete('-is:mergeable');
- }
- if (this.serverConfig) {
- getDocsBaseUrl(this.serverConfig, this.restApiService).then(baseUrl => {
- this.docBaseUrl = baseUrl;
- });
- }
- }
-
private valueChanged() {
this.inputVal = this.value;
}
+ private searchOperators() {
+ const set = new Set(SEARCH_OPERATORS_WITH_NEGATIONS_SET);
+ if (
+ this.mergeabilityComputationBehavior ===
+ MergeabilityComputationBehavior.API_REF_UPDATED_AND_CHANGE_REINDEX ||
+ this.mergeabilityComputationBehavior ===
+ MergeabilityComputationBehavior.REF_UPDATED_AND_CHANGE_REINDEX
+ ) {
+ set.add('is:mergeable');
+ set.add('-is:mergeable');
+ }
+ return set;
+ }
+
// private but used in test
computeHelpDocLink() {
// fallback to gerrit's official doc
let baseUrl =
- this.docBaseUrl ||
+ this.docsBaseUrl ||
'https://gerrit-review.googlesource.com/documentation/';
if (baseUrl.endsWith('/')) {
baseUrl = baseUrl.substring(0, baseUrl.length - 1);
@@ -308,7 +308,7 @@
if (!this.inputVal) return;
const trimmedInput = this.inputVal.trim();
if (trimmedInput) {
- const predefinedOpOnlyQuery = [...this.searchOperators].some(
+ const predefinedOpOnlyQuery = [...this.searchOperators()].some(
op => op.endsWith(':') && op === trimmedInput
);
if (predefinedOpOnlyQuery) {
@@ -365,7 +365,7 @@
default:
return Promise.resolve(
- [...this.searchOperators]
+ [...this.searchOperators()]
.filter(operator => operator.includes(input))
.map(operator => {
return {text: operator};
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
index 90574a2..db46c22 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
@@ -8,11 +8,9 @@
import {GrSearchBar} from './gr-search-bar';
import '../../../scripts/util';
import {mockPromise, waitUntil} from '../../../test/test-utils';
-import {_testOnly_clearDocsBaseUrlCache} from '../../../utils/url-util';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {
createChangeConfig,
- createGerritInfo,
createServerInfo,
} from '../../../test/test-data-generators';
import {MergeabilityComputationBehavior} from '../../../constants/constants';
@@ -176,14 +174,8 @@
suite('getSearchSuggestions', () => {
setup(async () => {
element = basicFixture.instantiate();
- element.serverConfig = {
- ...createServerInfo(),
- change: {
- ...createChangeConfig(),
- mergeability_computation_behavior:
- 'NEVER' as MergeabilityComputationBehavior,
- },
- };
+ element.mergeabilityComputationBehavior =
+ MergeabilityComputationBehavior.NEVER;
await element.updateComplete;
});
@@ -268,28 +260,21 @@
suite('doc url', () => {
setup(async () => {
- _testOnly_clearDocsBaseUrlCache();
element = basicFixture.instantiate();
- element.serverConfig = {
- ...createServerInfo(),
- gerrit: {
- ...createGerritInfo(),
- doc_url: 'https://doc.com/',
- },
- };
- await element.updateComplete;
});
- test('compute help doc url with correct path', () => {
- assert.equal(element.docBaseUrl, 'https://doc.com/');
+ test('compute help doc url with correct path', async () => {
+ element.docsBaseUrl = 'https://doc.com/';
+ await element.updateComplete;
assert.equal(
element.computeHelpDocLink(),
'https://doc.com/user-search.html'
);
});
- test('compute help doc url fallback to gerrit url', () => {
- element.docBaseUrl = null;
+ test('compute help doc url fallback to gerrit url', async () => {
+ element.docsBaseUrl = null;
+ await element.updateComplete;
assert.equal(
element.computeHelpDocLink(),
'https://gerrit-review.googlesource.com/documentation/' +
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
index edee3cb..bd3bd3e 100644
--- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
+++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
@@ -14,7 +14,10 @@
import {AutocompleteSuggestion} from '../../shared/gr-autocomplete/gr-autocomplete';
import {getAppContext} from '../../../services/app-context';
import {LitElement, html} from 'lit';
-import {customElement, property} from 'lit/decorators';
+import {customElement, property, state} from 'lit/decorators';
+import {subscribe} from '../../lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
const MAX_AUTOCOMPLETE_RESULTS = 10;
const SELF_EXPRESSION = 'self';
@@ -34,7 +37,7 @@
@property({type: String})
searchQuery = '';
- @property({type: Object})
+ @state()
serverConfig?: ServerInfo;
@property({type: String})
@@ -42,6 +45,19 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
+ );
+ }
+
override render() {
const accountSuggestions: SuggestionProvider = (predicate, expression) =>
this.fetchAccounts(predicate, expression);
@@ -57,7 +73,6 @@
.projectSuggestions=${projectSuggestions}
.groupSuggestions=${groupSuggestions}
.accountSuggestions=${accountSuggestions}
- .serverConfig=${this.serverConfig}
@handle-search=${(e: CustomEvent<SearchBarHandleSearchDetail>) => {
this.handleSearch(e);
}}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 6697113..be55097 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -22,14 +22,6 @@
import '../gr-patch-range-select/gr-patch-range-select';
import '../../change/gr-download-dialog/gr-download-dialog';
import '../../shared/gr-overlay/gr-overlay';
-import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {htmlTemplate} from './gr-diff-view_html';
-import {
- KeyboardShortcutMixin,
- Shortcut,
- ShortcutListener,
- ShortcutSection,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {
GeneratedWebLink,
GerritNav,
@@ -50,7 +42,6 @@
specialFilePathCompare,
} from '../../../utils/path-list-util';
import {changeBaseURL, changeIsOpen} from '../../../utils/change-util';
-import {customElement, observe, property} from '@polymer/decorators';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
import {
DropdownItem,
@@ -58,12 +49,10 @@
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
-import {GrDiffModeSelector} from '../../../embed/diff/gr-diff-mode-selector/gr-diff-mode-selector';
import {
BasePatchSetNum,
ChangeInfo,
CommitId,
- ConfigInfo,
EDIT,
FileInfo,
NumericChangeId,
@@ -85,7 +74,6 @@
ParsedChangeInfo,
} from '../../../types/types';
import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select';
-import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {GrDiffCursor} from '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
import {CommentSide, DiffViewMode, Side} from '../../../constants/constants';
import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog';
@@ -108,9 +96,10 @@
import {CursorMoveResult} from '../../../api/core';
import {isFalse, throttleWrap, until} from '../../../utils/async-util';
import {filter, take, switchMap} from 'rxjs/operators';
-import {combineLatest, Subscription} from 'rxjs';
+import {combineLatest} from 'rxjs';
import {
- listen,
+ Shortcut,
+ ShortcutSection,
shortcutsServiceToken,
} from '../../../services/shortcuts/shortcuts-service';
import {LoadingStatus} from '../../../models/change/change-model';
@@ -119,9 +108,17 @@
import {browserModelToken} from '../../../models/browser/browser-model';
import {commentsModelToken} from '../../../models/comments/comments-model';
import {changeModelToken} from '../../../models/change/change-model';
-import {resolve, DIPolymerElement} from '../../../models/dependency';
+import {resolve} from '../../../models/dependency';
import {BehaviorSubject} from 'rxjs';
-import {GrButton} from '../../shared/gr-button/gr-button';
+import {css, html, LitElement, PropertyValues} from 'lit';
+import {ShortcutController} from '../../lit/shortcut-controller';
+import {subscribe} from '../../lit/subscription-controller';
+import {customElement, property, query, state} from 'lit/decorators';
+import {configModelToken} from '../../../models/config/config-model';
+import {a11yStyles} from '../../../styles/gr-a11y-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {ifDefined} from 'lit/directives/if-defined';
+import {when} from 'lit/directives/when';
const LOADING_BLAME = 'Loading blame...';
const LOADED_BLAME = 'Blame loaded';
@@ -139,32 +136,8 @@
previous: string | null;
next: string | null;
}
-
-export interface GrDiffView {
- $: {
- diffHost: GrDiffHost;
- reviewed: HTMLInputElement;
- dropdown: GrDropdownList;
- diffPreferencesDialog: GrOverlay;
- applyFixDialog: GrApplyFixDialog;
- modeSelect: GrDiffModeSelector;
- downloadOverlay: GrOverlay;
- downloadDialog: GrDownloadDialog;
- toggleBlame: GrButton;
- diffPrefsContainer: HTMLElement;
- rangeSelect: HTMLElement;
- };
-}
-
-// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = KeyboardShortcutMixin(DIPolymerElement);
-
@customElement('gr-diff-view')
-export class GrDiffView extends base {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrDiffView extends LitElement {
/**
* Fired when the title of the page should change.
*
@@ -176,105 +149,138 @@
*
* @event show-alert
*/
+ @query('#diffHost')
+ diffHost?: GrDiffHost;
- @property({type: Object, observer: '_paramsChanged'})
- params?: AppElementParams;
+ @query('#reviewed')
+ reviewed?: HTMLInputElement;
+
+ @query('#downloadOverlay')
+ downloadOverlay?: GrOverlay;
+
+ @query('#downloadDialog')
+ downloadDialog?: GrDownloadDialog;
+
+ @query('#dropdown')
+ dropdown?: GrDropdownList;
+
+ @query('#applyFixDialog')
+ applyFixDialog?: GrApplyFixDialog;
+
+ @query('#diffPreferencesDialog')
+ diffPreferencesDialog?: GrOverlay;
+
+ /**
+ * URL params passed from the router.
+ * Use params getter/setter.
+ */
+ private _params?: AppElementParams;
@property({type: Object})
- _patchRange?: PatchRange;
+ get params() {
+ return this._params;
+ }
- @property({type: Object})
- _commitRange?: CommitRange;
+ set params(params: AppElementParams | undefined) {
+ if (this._params === params) return;
+ const oldParams = this._params;
+ this._params = params;
+ this.paramsChanged();
+ this.requestUpdate('params', oldParams);
+ }
- @property({type: Object})
- _change?: ParsedChangeInfo;
+ // Private but used in tests.
+ @state()
+ patchRange?: PatchRange;
- @property({type: Object})
- _changeComments?: ChangeComments;
+ // Private but used in tests.
+ @state()
+ commitRange?: CommitRange;
- @property({type: String})
- _changeNum?: NumericChangeId;
+ // Private but used in tests.
+ @state()
+ change?: ParsedChangeInfo;
- @property({type: Object})
- _diff?: DiffInfo;
+ // Private but used in tests.
+ @state()
+ changeComments?: ChangeComments;
- @property({
- type: Array,
- computed: '_formatFilesForDropdown(_files, _patchRange, _changeComments)',
- })
- _formattedFiles?: DropdownItem[];
+ // Private but used in tests.
+ @state()
+ changeNum?: NumericChangeId;
- @property({type: Array, computed: '_getSortedFileList(_files)'})
- _fileList?: string[];
+ // Private but used in tests.
+ @state()
+ diff?: DiffInfo;
- @property({type: Object})
- _files: Files = {sortedFileList: [], changeFilesByPath: {}};
+ // TODO: Move to using files-model.
+ // Private but used in tests.
+ @state()
+ files: Files = {sortedFileList: [], changeFilesByPath: {}};
- @property({type: Object, computed: '_getCurrentFile(_files, _path)'})
- _file?: FileInfo;
-
- @property({type: String, observer: '_pathChanged'})
+ // Private but used in tests
+ // Use path getter/setter.
_path?: string;
- @property({type: Number, computed: '_computeFileNum(_path, _formattedFiles)'})
- _fileNum?: number;
+ get path() {
+ return this._path;
+ }
- @property({type: Boolean})
- _loggedIn = false;
+ set path(path: string | undefined) {
+ if (this._path === path) return;
+ const oldPath = this._path;
+ this._path = path;
+ this.pathChanged();
+ this.requestUpdate('path', oldPath);
+ }
- @property({type: Boolean})
- _loading = true;
+ // Private but used in tests.
+ @state()
+ loggedIn = false;
+
+ // Private but used in tests.
+ @state()
+ loading = true;
@property({type: Object})
- _prefs?: DiffPreferencesInfo;
+ prefs?: DiffPreferencesInfo;
- @property({type: Object})
- _projectConfig?: ConfigInfo;
+ @state()
+ private serverConfig?: ServerInfo;
- @property({type: Object})
- _serverConfig?: ServerInfo;
+ // Private but used in tests.
+ @state()
+ userPrefs?: PreferencesInfo;
- @property({type: Object})
- _userPrefs?: PreferencesInfo;
+ @state()
+ private isImageDiff?: boolean;
- @property({type: Boolean})
- _isImageDiff?: boolean;
+ @state()
+ private editWeblinks?: GeneratedWebLink[];
- @property({type: Object})
- _editWeblinks?: GeneratedWebLink[];
+ @state()
+ private filesWeblinks?: FilesWebLinks;
- @property({type: Object})
- _filesWeblinks?: FilesWebLinks;
+ // Private but used in tests.
+ @state()
+ commentMap?: CommentMap;
- @property({type: Object})
- _commentMap?: CommentMap;
+ @state()
+ private commentSkips?: CommentSkips;
- @property({
- type: Object,
- computed: '_computeCommentSkips(_commentMap, _fileList, _path)',
- })
- _commentSkips?: CommentSkips;
+ // Private but used in tests.
+ @state()
+ isBlameLoaded?: boolean;
- @property({type: Boolean, computed: '_computeEditMode(_patchRange.*)'})
- _editMode?: boolean;
+ @state()
+ private isBlameLoading = false;
- @property({type: Boolean})
- _isBlameLoaded?: boolean;
+ @state()
+ private allPatchSets?: PatchSet[] = [];
- @property({type: Boolean})
- _isBlameLoading = false;
-
- @property({
- type: Array,
- computed: '_computeAllPatchSets(_change, _change.revisions.*)',
- })
- _allPatchSets?: PatchSet[] = [];
-
- @property({type: Object, computed: '_getRevisionInfo(_change)'})
- _revisionInfo?: RevisionInfoObj;
-
- @property({type: Number})
- _focusLineNum?: number;
+ // Private but used in tests.
+ @state()
+ focusLineNum?: number;
/** Called in disconnectedCallback. */
private cleanups: (() => void)[] = [];
@@ -282,67 +288,6 @@
// visible for testing
reviewedFiles = new Set<string>();
- override keyboardShortcuts(): ShortcutListener[] {
- return [
- listen(Shortcut.LEFT_PANE, _ => this.cursor?.moveLeft()),
- listen(Shortcut.RIGHT_PANE, _ => this.cursor?.moveRight()),
- listen(Shortcut.NEXT_LINE, _ => this.handleNextLine()),
- listen(Shortcut.PREV_LINE, _ => this.handlePrevLine()),
- listen(Shortcut.VISIBLE_LINE, _ => this.cursor?.moveToVisibleArea()),
- listen(Shortcut.NEXT_FILE_WITH_COMMENTS, _ =>
- this.moveToNextFileWithComment()
- ),
- listen(Shortcut.PREV_FILE_WITH_COMMENTS, _ =>
- this.moveToPreviousFileWithComment()
- ),
- listen(Shortcut.NEW_COMMENT, _ => this.handleNewComment()),
- listen(Shortcut.SAVE_COMMENT, _ => {}),
- listen(Shortcut.NEXT_FILE, _ => this.handleNextFile()),
- listen(Shortcut.PREV_FILE, _ => this.handlePrevFile()),
- listen(Shortcut.NEXT_CHUNK, _ => this.handleNextChunk()),
- listen(Shortcut.PREV_CHUNK, _ => this.handlePrevChunk()),
- listen(Shortcut.NEXT_COMMENT_THREAD, _ => this.handleNextCommentThread()),
- listen(Shortcut.PREV_COMMENT_THREAD, _ => this.handlePrevCommentThread()),
- listen(Shortcut.OPEN_REPLY_DIALOG, _ => this.handleOpenReplyDialog()),
- listen(Shortcut.TOGGLE_LEFT_PANE, _ => this.handleToggleLeftPane()),
- listen(Shortcut.OPEN_DOWNLOAD_DIALOG, _ =>
- this.handleOpenDownloadDialog()
- ),
- listen(Shortcut.UP_TO_CHANGE, _ => this.handleUpToChange()),
- listen(Shortcut.OPEN_DIFF_PREFS, _ => this.handleCommaKey()),
- listen(Shortcut.TOGGLE_DIFF_MODE, _ => this.handleToggleDiffMode()),
- listen(Shortcut.TOGGLE_FILE_REVIEWED, e => {
- if (this._throttledToggleFileReviewed) {
- this._throttledToggleFileReviewed(e);
- }
- }),
- listen(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, _ =>
- this.handleToggleAllDiffContext()
- ),
- listen(Shortcut.NEXT_UNREVIEWED_FILE, _ =>
- this.handleNextUnreviewedFile()
- ),
- listen(Shortcut.TOGGLE_BLAME, _ => this.handleToggleBlame()),
- listen(Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS, _ =>
- this._handleToggleHideAllCommentThreads()
- ),
- listen(Shortcut.OPEN_FILE_LIST, _ => this.handleOpenFileList()),
- listen(Shortcut.DIFF_AGAINST_BASE, _ => this.handleDiffAgainstBase()),
- listen(Shortcut.DIFF_AGAINST_LATEST, _ => this.handleDiffAgainstLatest()),
- listen(Shortcut.DIFF_BASE_AGAINST_LEFT, _ =>
- this.handleDiffBaseAgainstLeft()
- ),
- listen(Shortcut.DIFF_RIGHT_AGAINST_LATEST, _ =>
- this.handleDiffRightAgainstLatest()
- ),
- listen(Shortcut.DIFF_BASE_AGAINST_LATEST, _ =>
- this.handleDiffBaseAgainstLatest()
- ),
- listen(Shortcut.EXPAND_ALL_COMMENT_THREADS, _ => {}), // docOnly
- listen(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, _ => {}), // docOnly
- ];
- }
-
private readonly reporting = getAppContext().reportingService;
private readonly restApiService = getAppContext().restApiService;
@@ -364,88 +309,170 @@
private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
- _throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
+ private readonly getConfigModel = resolve(this, configModelToken);
- _onRenderHandler?: EventListener;
+ private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
- // visible for testing
+ @state()
cursor?: GrDiffCursor;
- private subscriptions: Subscription[] = [];
-
private connected$ = new BehaviorSubject(false);
- override connectedCallback() {
- super.connectedCallback();
- this.connected$.next(true);
- this._throttledToggleFileReviewed = throttleWrap(_ =>
- this.handleToggleFileReviewed()
+ private readonly shortcutsController = new ShortcutController(this);
+
+ constructor() {
+ super();
+ this.setupKeyboardShortcuts();
+ this.setupSubscriptions();
+ }
+
+ private setupKeyboardShortcuts() {
+ const listen = (shortcut: Shortcut, fn: (e: KeyboardEvent) => void) => {
+ this.shortcutsController.addAbstract(shortcut, fn);
+ };
+ listen(Shortcut.LEFT_PANE, _ => this.cursor?.moveLeft());
+ listen(Shortcut.RIGHT_PANE, _ => this.cursor?.moveRight());
+ listen(Shortcut.NEXT_LINE, _ => this.handleNextLine());
+ listen(Shortcut.PREV_LINE, _ => this.handlePrevLine());
+ listen(Shortcut.VISIBLE_LINE, _ => this.cursor?.moveToVisibleArea());
+ listen(Shortcut.NEXT_FILE_WITH_COMMENTS, _ =>
+ this.moveToNextFileWithComment()
);
- this._getLoggedIn().then(loggedIn => {
- this._loggedIn = loggedIn;
+ listen(Shortcut.PREV_FILE_WITH_COMMENTS, _ =>
+ this.moveToPreviousFileWithComment()
+ );
+ listen(Shortcut.NEW_COMMENT, _ => this.handleNewComment());
+ listen(Shortcut.SAVE_COMMENT, _ => {});
+ listen(Shortcut.NEXT_FILE, _ => this.handleNextFile());
+ listen(Shortcut.PREV_FILE, _ => this.handlePrevFile());
+ listen(Shortcut.NEXT_CHUNK, _ => this.handleNextChunk());
+ listen(Shortcut.PREV_CHUNK, _ => this.handlePrevChunk());
+ listen(Shortcut.NEXT_COMMENT_THREAD, _ => this.handleNextCommentThread());
+ listen(Shortcut.PREV_COMMENT_THREAD, _ => this.handlePrevCommentThread());
+ listen(Shortcut.OPEN_REPLY_DIALOG, _ => this.handleOpenReplyDialog());
+ listen(Shortcut.TOGGLE_LEFT_PANE, _ => this.handleToggleLeftPane());
+ listen(Shortcut.OPEN_DOWNLOAD_DIALOG, _ => this.handleOpenDownloadDialog());
+ listen(Shortcut.UP_TO_CHANGE, _ => this.handleUpToChange());
+ listen(Shortcut.OPEN_DIFF_PREFS, _ => this.handleCommaKey());
+ listen(Shortcut.TOGGLE_DIFF_MODE, _ => this.handleToggleDiffMode());
+ listen(Shortcut.TOGGLE_FILE_REVIEWED, e => {
+ if (this.throttledToggleFileReviewed) {
+ this.throttledToggleFileReviewed(e);
+ }
});
- this.restApiService.getConfig().then(config => {
- this._serverConfig = config;
- });
+ listen(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, _ =>
+ this.handleToggleAllDiffContext()
+ );
+ listen(Shortcut.NEXT_UNREVIEWED_FILE, _ => this.handleNextUnreviewedFile());
+ listen(Shortcut.TOGGLE_BLAME, _ => this.toggleBlame());
+ listen(Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS, _ =>
+ this.handleToggleHideAllCommentThreads()
+ );
+ listen(Shortcut.OPEN_FILE_LIST, _ => this.handleOpenFileList());
+ listen(Shortcut.DIFF_AGAINST_BASE, _ => this.handleDiffAgainstBase());
+ listen(Shortcut.DIFF_AGAINST_LATEST, _ => this.handleDiffAgainstLatest());
+ listen(Shortcut.DIFF_BASE_AGAINST_LEFT, _ =>
+ this.handleDiffBaseAgainstLeft()
+ );
+ listen(Shortcut.DIFF_RIGHT_AGAINST_LATEST, _ =>
+ this.handleDiffRightAgainstLatest()
+ );
+ listen(Shortcut.DIFF_BASE_AGAINST_LATEST, _ =>
+ this.handleDiffBaseAgainstLatest()
+ );
+ listen(Shortcut.EXPAND_ALL_COMMENT_THREADS, _ => {}); // docOnly
+ listen(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, _ => {}); // docOnly
+ }
- this.subscriptions.push(
- this.getCommentsModel().changeComments$.subscribe(changeComments => {
- this._changeComments = changeComments;
- })
+ private setupSubscriptions() {
+ subscribe(
+ this,
+ () => this.userModel.loggedIn$,
+ loggedIn => {
+ this.loggedIn = loggedIn;
+ }
);
-
- this.subscriptions.push(
- this.userModel.preferences$.subscribe(preferences => {
- this._userPrefs = preferences;
- })
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ this.serverConfig = config;
+ }
);
- this.subscriptions.push(
- this.userModel.diffPreferences$.subscribe(diffPreferences => {
- this._prefs = diffPreferences;
- })
+ subscribe(
+ this,
+ () => this.getCommentsModel().changeComments$,
+ changeComments => {
+ this.changeComments = changeComments;
+ }
);
- this.subscriptions.push(
- this.getChangeModel().change$.subscribe(change => {
- // The diff view is tied to a specfic change number, so don't update
- // _change to undefined.
- if (change) this._change = change;
- })
+ subscribe(
+ this,
+ () => this.userModel.preferences$,
+ preferences => {
+ this.userPrefs = preferences;
+ }
);
-
- this.subscriptions.push(
- this.getChangeModel().reviewedFiles$.subscribe(reviewedFiles => {
+ subscribe(
+ this,
+ () => this.userModel.diffPreferences$,
+ diffPreferences => {
+ this.prefs = diffPreferences;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ change => {
+ // The diff view is tied to a specific change number, so don't update
+ // change to undefined.
+ if (change) this.change = change;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().reviewedFiles$,
+ reviewedFiles => {
this.reviewedFiles = new Set(reviewedFiles) ?? new Set();
- })
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().diffPath$,
+ path => (this.path = path)
);
- this.subscriptions.push(
- this.getChangeModel().diffPath$.subscribe(path => (this._path = path))
- );
-
- this.subscriptions.push(
- combineLatest(
- this.getChangeModel().diffPath$,
- this.getChangeModel().reviewedFiles$
- ).subscribe(([path, files]) => {
- this.$.reviewed.checked = !!path && !!files && files.includes(path);
- })
+ subscribe(
+ this,
+ () =>
+ combineLatest([
+ this.getChangeModel().diffPath$,
+ this.getChangeModel().reviewedFiles$,
+ ]),
+ ([path, files]) => {
+ this.updateComplete.then(() => {
+ assertIsDefined(this.reviewed, 'reviewed');
+ this.reviewed.checked = !!path && !!files && files.includes(path);
+ });
+ }
);
// When user initially loads the diff view, we want to autmatically mark
// the file as reviewed if they have it enabled. We can't observe these
// properties since the method will be called anytime a property updates
// but we only want to call this on the initial load.
- this.subscriptions.push(
- this.getChangeModel()
- .diffPath$.pipe(
+ subscribe(
+ this,
+ () =>
+ this.getChangeModel().diffPath$.pipe(
filter(diffPath => !!diffPath),
switchMap(() =>
- combineLatest(
+ combineLatest([
this.getChangeModel().patchNum$,
this.routerModel.routerView$,
this.userModel.diffPreferences$,
- this.getChangeModel().reviewedFiles$
- ).pipe(
+ this.getChangeModel().reviewedFiles$,
+ ]).pipe(
filter(
([patchNum, routerView, diffPrefs, reviewedFiles]) =>
!!patchNum &&
@@ -456,245 +483,720 @@
take(1)
)
)
- )
- .subscribe(([patchNum, _routerView, diffPrefs]) => {
- this.setReviewedStatus(patchNum!, diffPrefs);
- })
+ ),
+ ([patchNum, _routerView, diffPrefs]) => {
+ this.setReviewedStatus(patchNum!, diffPrefs);
+ }
);
- this.subscriptions.push(
- this.getChangeModel().diffPath$.subscribe(path => (this._path = path))
+ subscribe(
+ this,
+ () => this.getChangeModel().diffPath$,
+ path => (this.path = path)
+ );
+ }
+
+ static override get styles() {
+ return [
+ a11yStyles,
+ sharedStyles,
+ css`
+ :host {
+ display: block;
+ background-color: var(--view-background-color);
+ }
+ .hidden {
+ display: none;
+ }
+ gr-patch-range-select {
+ display: block;
+ }
+ gr-diff {
+ border: none;
+ }
+ .stickyHeader {
+ background-color: var(--view-background-color);
+ position: sticky;
+ top: 0;
+ /* TODO(dhruvsri): This is required only because of 'position:relative' in
+ <gr-diff-highlight> (which could maybe be removed??). */
+ z-index: 1;
+ box-shadow: var(--elevation-level-1);
+ /* This is just for giving the box-shadow some space. */
+ margin-bottom: 2px;
+ }
+ header,
+ .subHeader {
+ align-items: center;
+ display: flex;
+ justify-content: space-between;
+ }
+ header {
+ padding: var(--spacing-s) var(--spacing-xl);
+ border-bottom: 1px solid var(--border-color);
+ }
+ .changeNumberColon {
+ color: transparent;
+ }
+ .headerSubject {
+ margin-right: var(--spacing-m);
+ font-weight: var(--font-weight-bold);
+ }
+ .patchRangeLeft {
+ align-items: center;
+ display: flex;
+ }
+ .navLink:not([href]) {
+ color: var(--deemphasized-text-color);
+ }
+ .navLinks {
+ align-items: center;
+ display: flex;
+ white-space: nowrap;
+ }
+ .navLink {
+ padding: 0 var(--spacing-xs);
+ }
+ .reviewed {
+ display: inline-block;
+ margin: 0 var(--spacing-xs);
+ vertical-align: top;
+ position: relative;
+ top: 8px;
+ }
+ .jumpToFileContainer {
+ display: inline-block;
+ word-break: break-all;
+ }
+ .mobile {
+ display: none;
+ }
+ gr-button {
+ padding: var(--spacing-s) 0;
+ text-decoration: none;
+ }
+ .loading {
+ color: var(--deemphasized-text-color);
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h1);
+ font-weight: var(--font-weight-h1);
+ line-height: var(--line-height-h1);
+ height: 100%;
+ padding: var(--spacing-l);
+ text-align: center;
+ }
+ .subHeader {
+ background-color: var(--background-color-secondary);
+ flex-wrap: wrap;
+ padding: 0 var(--spacing-l);
+ }
+ .prefsButton {
+ text-align: right;
+ }
+ .editMode .hideOnEdit {
+ display: none;
+ }
+ .blameLoader,
+ .fileNum {
+ display: none;
+ }
+ .blameLoader.show,
+ .fileNum.show,
+ .download,
+ .preferences,
+ .rightControls {
+ align-items: center;
+ display: flex;
+ }
+ .diffModeSelector,
+ .editButton {
+ align-items: center;
+ display: flex;
+ }
+ .diffModeSelector span,
+ .editButton span {
+ margin-right: var(--spacing-xs);
+ }
+ .diffModeSelector.hide,
+ .separator.hide {
+ display: none;
+ }
+ .editButtona a {
+ text-decoration: none;
+ }
+ @media screen and (max-width: 50em) {
+ header {
+ padding: var(--spacing-s) var(--spacing-l);
+ }
+ .dash {
+ display: none;
+ }
+ .desktop {
+ display: none;
+ }
+ .fileNav {
+ align-items: flex-start;
+ display: flex;
+ margin: 0 var(--spacing-xs);
+ }
+ .fullFileName {
+ display: block;
+ font-style: italic;
+ min-width: 50%;
+ padding: 0 var(--spacing-xxs);
+ text-align: center;
+ width: 100%;
+ word-wrap: break-word;
+ }
+ .reviewed {
+ vertical-align: -1px;
+ }
+ .mobileNavLink {
+ color: var(--primary-text-color);
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h2);
+ font-weight: var(--font-weight-h2);
+ line-height: var(--line-height-h2);
+ text-decoration: none;
+ }
+ .mobileNavLink:not([href]) {
+ color: var(--deemphasized-text-color);
+ }
+ .jumpToFileContainer {
+ display: block;
+ width: 100%;
+ word-break: break-all;
+ }
+ /* prettier formatter removes semi-colons after css mixins. */
+ /* prettier-ignore */
+ gr-dropdown-list {
+ width: 100%;
+ --gr-select-style-width: 100%;
+ --gr-select-style-display: block;
+ --native-select-style-width: 100%;
+ }
+ }
+ :host(.hideComments) {
+ --gr-comment-thread-display: none;
+ }
+ `,
+ ];
+ }
+
+ override connectedCallback() {
+ super.connectedCallback();
+ this.connected$.next(true);
+ this.throttledToggleFileReviewed = throttleWrap(_ =>
+ this.handleToggleFileReviewed()
);
this.addEventListener('open-fix-preview', e => this.onOpenFixPreview(e));
this.cursor = new GrDiffCursor();
- this._onRenderHandler = (_: Event) => {
- // We have to wait until render because at the time of connectedCallback,
- // gr-diff-host has not been rendered yet.
- this.cursor?.replaceDiffs([this.$.diffHost]);
- this.cursor?.reInitCursor();
- };
- this.$.diffHost.addEventListener('render', this._onRenderHandler);
this.cleanups.push(
- addGlobalShortcut(
- {key: Key.ESC},
- _ => (this.$.diffHost.displayLine = false)
- )
+ addGlobalShortcut({key: Key.ESC}, _ => {
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.displayLine = false;
+ })
);
}
override disconnectedCallback() {
this.cursor?.dispose();
- if (this._onRenderHandler) {
- this.$.diffHost.removeEventListener('render', this._onRenderHandler);
- this._onRenderHandler = undefined;
- }
for (const cleanup of this.cleanups) cleanup();
this.cleanups = [];
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
this.connected$.next(false);
super.disconnectedCallback();
}
+ protected override willUpdate(changedProperties: PropertyValues) {
+ super.willUpdate(changedProperties);
+ if (changedProperties.has('change')) {
+ this.allPatchSets = computeAllPatchSets(this.change);
+ }
+ if (
+ changedProperties.has('commentMap') ||
+ changedProperties.has('files') ||
+ changedProperties.has('path')
+ ) {
+ this.commentSkips = this.computeCommentSkips(
+ this.commentMap,
+ this.files?.sortedFileList,
+ this.path
+ );
+ }
+
+ if (
+ changedProperties.has('changeNum') ||
+ changedProperties.has('changeComments') ||
+ changedProperties.has('patchRange')
+ ) {
+ this.fetchFiles();
+ }
+ }
+
+ protected override updated(changedProperties: PropertyValues): void {
+ super.updated(changedProperties);
+ assertIsDefined(this.diffHost, 'diffHost');
+ if (
+ changedProperties.has('change') ||
+ changedProperties.has('changeNum') ||
+ changedProperties.has('cursor') ||
+ changedProperties.has('commitRange') ||
+ changedProperties.has('path') ||
+ changedProperties.has('patchRange') ||
+ changedProperties.has('files')
+ ) {
+ // We have to wait until render because at the time of updated,
+ // gr-diff-host has not been rendered yet.
+ this.diffHost.updateComplete.then(() => {
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.cursor?.replaceDiffs([this.diffHost]);
+ this.cursor?.reInitCursor();
+ });
+ }
+ if (
+ changedProperties.has('changeComments') ||
+ changedProperties.has('path') ||
+ changedProperties.has('patchRange') ||
+ changedProperties.has('files')
+ ) {
+ if (this.changeComments && this.path && this.patchRange) {
+ assertIsDefined(this.diffHost);
+ const file = this.files?.changeFilesByPath
+ ? this.files.changeFilesByPath[this.path]
+ : undefined;
+ this.diffHost.updateComplete.then(() => {
+ assertIsDefined(this.path);
+ assertIsDefined(this.patchRange);
+ assertIsDefined(this.diffHost);
+ assertIsDefined(this.changeComments);
+ this.diffHost.threads = this.changeComments.getThreadsBySideForFile(
+ {path: this.path, basePath: file?.old_path},
+ this.patchRange
+ );
+ });
+ }
+ }
+ }
+
+ override render() {
+ const file = this.getFileRange();
+ return html`
+ ${this.renderStickyHeader()}
+ <div class="loading" ?hidden=${!this.loading}>Loading...</div>
+ <h2 class="assistive-tech-only">Diff view</h2>
+ <gr-diff-host
+ id="diffHost"
+ ?hidden=${this.loading}
+ .changeNum=${this.changeNum}
+ .change=${this.change}
+ .commitRange=${this.commitRange}
+ .patchRange=${this.patchRange}
+ .file=${file}
+ .path=${this.path}
+ .projectName=${this.change?.project}
+ @is-blame-loaded-changed=${this.onIsBlameLoadedChanged}
+ @comment-anchor-tap=${this.onLineSelected}
+ @line-selected=${this.onLineSelected}
+ @diff-changed=${this.onDiffChanged}
+ @edit-weblinks-changed=${this.onEditWeblinksChanged}
+ @files-weblinks-changed=${this.onFilesWeblinksChanged}
+ @is-image-diff-changed=${this.onIsImageDiffChanged}
+ >
+ </gr-diff-host>
+ ${this.renderDialogs()}
+ `;
+ }
+
+ private renderStickyHeader() {
+ return html` <div
+ class="stickyHeader ${this.computeEditMode() ? 'editMode' : ''}"
+ >
+ <h1 class="assistive-tech-only">
+ Diff of ${this.path ? computeTruncatedPath(this.path) : ''}
+ </h1>
+ <header>${this.renderHeader()}</header>
+ <div class="subHeader">
+ ${this.renderPatchRangeLeft()} ${this.renderRightControls()}
+ </div>
+ <div class="fileNav mobile">
+ <a class="mobileNavLink" href=${ifDefined(this.computeNavLinkURL(-1))}
+ ><</a
+ >
+ <div class="fullFileName mobile">${computeDisplayPath(this.path)}</div>
+ <a class="mobileNavLink" href=${ifDefined(this.computeNavLinkURL(1))}
+ >></a
+ >
+ </div>
+ </div>`;
+ }
+
+ private renderHeader() {
+ const formattedFiles = this.formatFilesForDropdown();
+ const fileNum = this.computeFileNum(formattedFiles);
+ const fileNumClass = this.computeFileNumClass(fileNum, formattedFiles);
+ return html` <div>
+ <a href=${this.getChangePath()}>${this.changeNum}</a
+ ><span class="changeNumberColon">:</span>
+ <span class="headerSubject">${this.change?.subject}</span>
+ <input
+ id="reviewed"
+ class="reviewed hideOnEdit"
+ type="checkbox"
+ ?hidden=${!this.loggedIn}
+ title="Toggle reviewed status of file"
+ aria-label="file reviewed"
+ @change=${this.handleReviewedChange}
+ />
+ <div class="jumpToFileContainer">
+ <gr-dropdown-list
+ id="dropdown"
+ .value=${this.path}
+ .items=${formattedFiles}
+ initial-count="75"
+ show-copy-for-trigger-text
+ @value-change=${this.handleFileChange}
+ ></gr-dropdown-list>
+ </div>
+ </div>
+ <div class="navLinks desktop">
+ <span class="fileNum ${ifDefined(fileNumClass)}">
+ File ${fileNum} of ${formattedFiles.length}
+ <span class="separator"></span>
+ </span>
+ <a
+ class="navLink"
+ title=${this.createTitle(
+ Shortcut.PREV_FILE,
+ ShortcutSection.NAVIGATION
+ )}
+ href=${ifDefined(this.computeNavLinkURL(-1))}
+ >Prev</a
+ >
+ <span class="separator"></span>
+ <a
+ class="navLink"
+ title=${this.createTitle(
+ Shortcut.UP_TO_CHANGE,
+ ShortcutSection.NAVIGATION
+ )}
+ href=${this.getChangePath()}
+ >Up</a
+ >
+ <span class="separator"></span>
+ <a
+ class="navLink"
+ title=${this.createTitle(
+ Shortcut.NEXT_FILE,
+ ShortcutSection.NAVIGATION
+ )}
+ href=${ifDefined(this.computeNavLinkURL(1))}
+ >Next</a
+ >
+ </div>`;
+ }
+
+ private renderPatchRangeLeft() {
+ const revisionInfo = this.change
+ ? new RevisionInfoObj(this.change)
+ : undefined;
+ return html` <div class="patchRangeLeft">
+ <gr-patch-range-select
+ id="rangeSelect"
+ .changeNum=${this.changeNum}
+ .patchNum=${this.patchRange?.patchNum}
+ .basePatchNum=${this.patchRange?.basePatchNum}
+ .filesWeblinks=${this.filesWeblinks}
+ .availablePatches=${this.allPatchSets}
+ .revisions=${this.change?.revisions}
+ .revisionInfo=${revisionInfo}
+ @patch-range-change=${this.handlePatchChange}
+ >
+ </gr-patch-range-select>
+ <span class="download desktop">
+ <span class="separator"></span>
+ <gr-dropdown
+ link=""
+ down-arrow=""
+ .items=${this.computeDownloadDropdownLinks()}
+ horizontal-align="left"
+ >
+ <span class="downloadTitle"> Download </span>
+ </gr-dropdown>
+ </span>
+ </div>`;
+ }
+
+ private renderRightControls() {
+ const blameLoaderClass =
+ !isMagicPath(this.path) && !this.isImageDiff ? 'show' : '';
+ const blameToggleLabel =
+ this.isBlameLoaded && !this.isBlameLoading ? 'Hide blame' : 'Show blame';
+ const diffModeSelectorClass = !this.diff || this.diff.binary ? 'hide' : '';
+ return html` <div class="rightControls">
+ <span class="blameLoader ${blameLoaderClass}">
+ <gr-button
+ link=""
+ id="toggleBlame"
+ title=${this.createTitle(
+ Shortcut.TOGGLE_BLAME,
+ ShortcutSection.DIFFS
+ )}
+ ?disabled=${this.isBlameLoading}
+ @click=${this.toggleBlame}
+ >${blameToggleLabel}</gr-button
+ >
+ </span>
+ ${when(
+ this.computeCanEdit(),
+ () => html`
+ <span class="separator"></span>
+ <span class="editButton">
+ <gr-button
+ link=""
+ title="Edit current file"
+ @click=${this.goToEditFile}
+ >edit</gr-button
+ >
+ </span>
+ `
+ )}
+ ${when(
+ this.computeShowEditLinks(),
+ () => html`
+ <span class="separator"></span>
+ ${this.editWeblinks!.map(
+ weblink => html`
+ <a target="_blank" href=${ifDefined(weblink.url)}
+ >${weblink.name}</a
+ >
+ `
+ )}
+ `
+ )}
+ <span class="separator"></span>
+ <div class="diffModeSelector ${diffModeSelectorClass}">
+ <span>Diff view:</span>
+ <gr-diff-mode-selector
+ id="modeSelect"
+ .saveOnChange=${this.loggedIn}
+ show-tooltip-below
+ ></gr-diff-mode-selector>
+ </div>
+ ${when(
+ this.loggedIn && this.prefs,
+ () => html`
+ <span id="diffPrefsContainer">
+ <span class="preferences desktop">
+ <gr-tooltip-content
+ has-tooltip=""
+ position-below=""
+ title="Diff preferences"
+ >
+ <gr-button
+ link=""
+ class="prefsButton"
+ @click=${(e: Event) => this.handlePrefsTap(e)}
+ ><iron-icon icon="gr-icons:settings"></iron-icon
+ ></gr-button>
+ </gr-tooltip-content>
+ </span>
+ </span>
+ `
+ )}
+ <gr-endpoint-decorator name="annotation-toggler">
+ <span hidden="" id="annotation-span">
+ <label for="annotation-checkbox" id="annotation-label"></label>
+ <iron-input>
+ <input
+ is="iron-input"
+ type="checkbox"
+ id="annotation-checkbox"
+ disabled=""
+ />
+ </iron-input>
+ </span>
+ </gr-endpoint-decorator>
+ </div>`;
+ }
+
+ private renderDialogs() {
+ return html` <gr-apply-fix-dialog
+ id="applyFixDialog"
+ .change=${this.change}
+ .changeNum=${this.changeNum}
+ >
+ </gr-apply-fix-dialog>
+ <gr-diff-preferences-dialog
+ id="diffPreferencesDialog"
+ @reload-diff-preference=${this.handleReloadingDiffPreference}
+ >
+ </gr-diff-preferences-dialog>
+ <gr-overlay id="downloadOverlay">
+ <gr-download-dialog
+ id="downloadDialog"
+ .change=${this.change}
+ .patchNum=${this.patchRange?.patchNum}
+ .config=${this.serverConfig?.download}
+ @close=${this.handleDownloadDialogClose}
+ ></gr-download-dialog>
+ </gr-overlay>`;
+ }
+
/**
* Set initial review status of the file.
* automatically mark the file as reviewed if manual review is not set.
*/
-
- async setReviewedStatus(
+ setReviewedStatus(
patchNum: RevisionPatchSetNum,
diffPrefs: DiffPreferencesInfo
) {
- const loggedIn = await this._getLoggedIn();
- if (!loggedIn) return;
+ if (!this.loggedIn) return;
if (!diffPrefs.manual_review) {
this.setReviewed(true, patchNum);
}
}
- @observe('_changeComments', '_path', '_patchRange')
- computeThreads(
- changeComments?: ChangeComments,
- path?: string,
- patchRange?: PatchRange
- ) {
- if (
- changeComments === undefined ||
- path === undefined ||
- patchRange === undefined
- ) {
- return;
- }
- // TODO(dhruvsri): check if basePath should be set here
- this.$.diffHost.threads = changeComments.getThreadsBySideForFile(
- {path},
- patchRange
- );
- }
-
- _getLoggedIn(): Promise<boolean> {
- return this.restApiService.getLoggedIn();
- }
-
- @observe('_change.project')
- _getProjectConfig(project?: RepoName) {
- if (!project) return;
- return this.restApiService.getProjectConfig(project).then(config => {
- this._projectConfig = config;
- });
- }
-
- _getSortedFileList(files?: Files) {
- if (!files) return [];
- return files.sortedFileList;
- }
-
- _getCurrentFile(files?: Files, path?: string) {
- if (!files || !path) return;
- const fileInfo = files.changeFilesByPath[path];
- const fileRange: FileRange = {path};
+ private getFileRange() {
+ if (!this.files || !this.path) return;
+ const fileInfo = this.files.changeFilesByPath[this.path];
+ const fileRange: FileRange = {path: this.path};
if (fileInfo && fileInfo.old_path) {
fileRange.basePath = fileInfo.old_path;
}
return fileRange;
}
- @observe('_changeNum', '_patchRange.*', '_changeComments')
- _getFiles(
- changeNum: NumericChangeId,
- patchRangeRecord: PolymerDeepPropertyChange<PatchRange, PatchRange>,
- changeComments: ChangeComments
- ) {
+ // Private but used in tests.
+ fetchFiles() {
// Polymer 2: check for undefined
- if (
- [changeNum, patchRangeRecord, patchRangeRecord.base, changeComments].some(
- arg => arg === undefined
- )
- ) {
+ if (!this.changeNum || !this.patchRange || !this.changeComments) {
return Promise.resolve();
}
- if (!patchRangeRecord.base.patchNum) {
+ if (!this.patchRange.patchNum) {
return Promise.resolve();
}
- const patchRange = patchRangeRecord.base;
return this.restApiService
- .getChangeFiles(changeNum, patchRange)
+ .getChangeFiles(this.changeNum, this.patchRange)
.then(changeFiles => {
if (!changeFiles) return;
- const commentedPaths = changeComments.getPaths(patchRange);
+ const commentedPaths = this.changeComments!.getPaths(this.patchRange);
const files = {...changeFiles};
addUnmodifiedFiles(files, commentedPaths);
- this._files = {
+ this.files = {
sortedFileList: Object.keys(files).sort(specialFilePathCompare),
changeFilesByPath: files,
};
});
}
- _handleReviewedChange(e: Event) {
- this.setReviewed(
- ((dom(e) as EventApi).rootTarget as HTMLInputElement).checked
- );
+ private handleReviewedChange(e: Event) {
+ const input = e.target as HTMLInputElement;
+ this.setReviewed(input.checked ?? false);
}
// Private but used in tests.
setReviewed(
reviewed: boolean,
- patchNum: RevisionPatchSetNum | undefined = this._patchRange?.patchNum
+ patchNum: RevisionPatchSetNum | undefined = this.patchRange?.patchNum
) {
- if (this._editMode) return;
- if (!patchNum || !this._path || !this._changeNum) return;
- const path = this._path;
+ if (this.computeEditMode()) return;
+ if (!patchNum || !this.path || !this.changeNum) return;
// if file is already reviewed then do not make a saveReview request
- if (this.reviewedFiles.has(path) && reviewed) return;
+ if (this.reviewedFiles.has(this.path) && reviewed) return;
this.getChangeModel().setReviewedFilesStatus(
- this._changeNum,
+ this.changeNum,
patchNum,
- path,
+ this.path,
reviewed
);
}
// Private but used in tests.
handleToggleFileReviewed() {
- this.setReviewed(!this.$.reviewed.checked);
+ assertIsDefined(this.reviewed);
+ this.setReviewed(!this.reviewed.checked);
}
private handlePrevLine() {
- this.$.diffHost.displayLine = true;
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.displayLine = true;
this.cursor?.moveUp();
}
private onOpenFixPreview(e: OpenFixPreviewEvent) {
- this.$.applyFixDialog.open(e);
+ assertIsDefined(this.applyFixDialog, 'applyFixDialog');
+ this.applyFixDialog.open(e);
}
- _onIsBlameLoadedchanged(e: ValueChangedEvent<boolean>) {
- this._isBlameLoaded = e.detail.value;
+ private onIsBlameLoadedChanged(e: ValueChangedEvent<boolean>) {
+ this.isBlameLoaded = e.detail.value;
}
- _onDiffChanged(e: ValueChangedEvent<DiffInfo>) {
- this._diff = e.detail.value;
+ private onDiffChanged(e: ValueChangedEvent<DiffInfo>) {
+ this.diff = e.detail.value;
}
- _onEditWeblinksChanged(e: ValueChangedEvent<GeneratedWebLink[] | undefined>) {
- this._editWeblinks = e.detail.value;
+ private onEditWeblinksChanged(
+ e: ValueChangedEvent<GeneratedWebLink[] | undefined>
+ ) {
+ this.editWeblinks = e.detail.value;
}
- _onFilesWeblinksChanged(e: ValueChangedEvent<FilesWebLinks | undefined>) {
- this._filesWeblinks = e.detail.value;
+ private onFilesWeblinksChanged(
+ e: ValueChangedEvent<FilesWebLinks | undefined>
+ ) {
+ this.filesWeblinks = e.detail.value;
}
- _onIsImageDiffChanged(e: ValueChangedEvent<boolean>) {
- this._isImageDiff = e.detail.value;
+ private onIsImageDiffChanged(e: ValueChangedEvent<boolean>) {
+ this.isImageDiff = e.detail.value;
}
private handleNextLine() {
- this.$.diffHost.displayLine = true;
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.displayLine = true;
this.cursor?.moveDown();
}
// Private but used in tests.
moveToPreviousFileWithComment() {
- if (!this._commentSkips) return;
- if (!this._change) return;
- if (!this._patchRange?.patchNum) return;
+ if (!this.commentSkips) return;
+ if (!this.change) return;
+ if (!this.patchRange?.patchNum) return;
// If there is no previous diff with comments, then return to the change
// view.
- if (!this._commentSkips.previous) {
+ if (!this.commentSkips.previous) {
this.navToChangeView();
return;
}
GerritNav.navigateToDiff(
- this._change,
- this._commentSkips.previous,
- this._patchRange.patchNum,
- this._patchRange.basePatchNum
+ this.change,
+ this.commentSkips.previous,
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum
);
}
// Private but used in tests.
moveToNextFileWithComment() {
- if (!this._commentSkips) return;
- if (!this._change) return;
- if (!this._patchRange?.patchNum) return;
+ if (!this.commentSkips) return;
+ if (!this.change) return;
+ if (!this.patchRange?.patchNum) return;
// If there is no next diff with comments, then return to the change view.
- if (!this._commentSkips.next) {
+ if (!this.commentSkips.next) {
this.navToChangeView();
return;
}
GerritNav.navigateToDiff(
- this._change,
- this._commentSkips.next,
- this._patchRange.patchNum,
- this._patchRange.basePatchNum
+ this.change,
+ this.commentSkips.next,
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum
);
}
@@ -704,15 +1206,15 @@
}
private handlePrevFile() {
- if (!this._path) return;
- if (!this._fileList) return;
- this.navToFile(this._path, this._fileList, -1);
+ if (!this.path) return;
+ if (!this.files?.sortedFileList) return;
+ this.navToFile(this.files.sortedFileList, -1);
}
private handleNextFile() {
- if (!this._path) return;
- if (!this._fileList) return;
- this.navToFile(this._path, this._fileList, 1);
+ if (!this.path) return;
+ if (!this.files?.sortedFileList) return;
+ this.navToFile(this.files.sortedFileList, 1);
}
private handleNextChunk() {
@@ -755,16 +1257,16 @@
}
private navigateToUnreviewedFile(direction: string) {
- if (!this._path) return;
- if (!this._fileList) return;
+ if (!this.path) return;
+ if (!this.files?.sortedFileList) return;
if (!this.reviewedFiles) return;
// Ensure that the currently viewed file always appears in unreviewedFiles
// so we resolve the right "next" file.
- const unreviewedFiles = this._fileList.filter(
- file => file === this._path || !this.reviewedFiles.has(file)
+ const unreviewedFiles = this.files.sortedFileList.filter(
+ file => file === this.path || !this.reviewedFiles.has(file)
);
- this.navToFile(this._path, unreviewedFiles, direction === 'next' ? 1 : -1);
+ this.navToFile(unreviewedFiles, direction === 'next' ? 1 : -1);
}
private handlePrevChunk() {
@@ -780,30 +1282,31 @@
// Similar to gr-change-view.handleOpenReplyDialog
private handleOpenReplyDialog() {
- this._getLoggedIn().then(isLoggedIn => {
- if (!isLoggedIn) {
- fireEvent(this, 'show-auth-required');
- return;
- }
- this.navToChangeView(true);
- });
+ if (!this.loggedIn) {
+ fireEvent(this, 'show-auth-required');
+ return;
+ }
+ this.navToChangeView(true);
}
private handleToggleLeftPane() {
- this.$.diffHost.toggleLeftDiff();
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.toggleLeftDiff();
}
private handleOpenDownloadDialog() {
- this.$.downloadOverlay.open().then(() => {
- this.$.downloadOverlay.setFocusStops(
- this.$.downloadDialog.getFocusStops()
- );
- this.$.downloadDialog.focus();
+ assertIsDefined(this.downloadOverlay, 'downloadOverlay');
+ this.downloadOverlay.open().then(() => {
+ assertIsDefined(this.downloadOverlay, 'downloadOverlay');
+ assertIsDefined(this.downloadDialog, 'downloadOverlay');
+ this.downloadOverlay.setFocusStops(this.downloadDialog.getFocusStops());
+ this.downloadDialog.focus();
});
}
- _handleDownloadDialogClose() {
- this.$.downloadOverlay.close();
+ private handleDownloadDialogClose() {
+ assertIsDefined(this.downloadOverlay, 'downloadOverlay');
+ this.downloadOverlay.close();
}
private handleUpToChange() {
@@ -811,14 +1314,15 @@
}
private handleCommaKey() {
- if (!this._loggedIn) return;
- this.$.diffPreferencesDialog.open();
+ if (!this.loggedIn) return;
+ assertIsDefined(this.diffPreferencesDialog, 'diffPreferencesDialog');
+ this.diffPreferencesDialog.open();
}
// Private but used in tests.
handleToggleDiffMode() {
- if (!this._userPrefs) return;
- if (this._userPrefs.diff_view === DiffViewMode.SIDE_BY_SIDE) {
+ if (!this.userPrefs) return;
+ if (this.userPrefs.diff_view === DiffViewMode.SIDE_BY_SIDE) {
this.userModel.updatePreferences({diff_view: DiffViewMode.UNIFIED});
} else {
this.userModel.updatePreferences({
@@ -829,34 +1333,33 @@
// Private but used in tests.
navToChangeView(openReplyDialog = false) {
- if (!this._changeNum || !this._patchRange?.patchNum) {
+ if (!this.changeNum || !this.patchRange?.patchNum) {
return;
}
- this._navigateToChange(
- this._change,
- this._patchRange,
- this._change && this._change.revisions,
+ this.navigateToChange(
+ this.change,
+ this.patchRange,
+ this.change && this.change.revisions,
openReplyDialog
);
}
// Private but used in tests.
navToFile(
- path: string,
fileList: string[],
direction: -1 | 1,
navigateToFirstComment?: boolean
) {
- const newPath = this.getNavLinkPath(path, fileList, direction);
+ const newPath = this.getNavLinkPath(fileList, direction);
if (!newPath) return;
- if (!this._change) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.patchRange) return;
if (newPath.up) {
- this._navigateToChange(
- this._change,
- this._patchRange,
- this._change && this._change.revisions
+ this.navigateToChange(
+ this.change,
+ this.patchRange,
+ this.change && this.change.revisions
);
return;
}
@@ -864,64 +1367,52 @@
if (!newPath.path) return;
let lineNum;
if (navigateToFirstComment)
- lineNum = this._changeComments?.getCommentsForPath(
+ lineNum = this.changeComments?.getCommentsForPath(
newPath.path,
- this._patchRange
+ this.patchRange
)?.[0].line;
GerritNav.navigateToDiff(
- this._change,
+ this.change,
newPath.path,
- this._patchRange.patchNum,
- this._patchRange.basePatchNum,
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum,
lineNum
);
}
/**
- * @param path The path of the current file being shown.
- * @param fileList The list of files in this change and
- * patch range.
* @param direction Either 1 (next file) or -1 (prev file).
* @return The next URL when proceeding in the specified
* direction.
*/
- _computeNavLinkURL(
- change?: ChangeInfo,
- path?: string,
- fileList?: string[],
- direction?: -1 | 1
- ) {
- if (!change) return null;
- if (!path) return null;
- if (!fileList) return null;
- if (!direction) return null;
+ private computeNavLinkURL(direction?: -1 | 1) {
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.files?.sortedFileList) return;
+ if (!direction) return;
- const newPath = this.getNavLinkPath(path, fileList, direction);
+ const newPath = this.getNavLinkPath(this.files.sortedFileList, direction);
if (!newPath) {
- return null;
+ return;
}
if (newPath.up) {
- return this._getChangePath(
- this._change,
- this._patchRange,
- this._change && this._change.revisions
- );
+ return this.getChangePath();
}
- return this._getDiffUrl(this._change, this._patchRange, newPath.path);
+ return this.getDiffUrl(this.change, this.patchRange, newPath.path);
}
- _goToEditFile() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ private goToEditFile() {
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
// TODO(taoalpha): add a shortcut for editing
const cursorAddress = this.cursor?.getAddress();
const editUrl = GerritNav.getEditUrlForDiff(
- this._change,
- this._path,
- this._patchRange.patchNum,
+ this.change,
+ this.path,
+ this.patchRange.patchNum,
cursorAddress?.number
);
GerritNav.navigateToRelativeUrl(editUrl);
@@ -942,12 +1433,12 @@
* patch range.
* @param direction Either 1 (next file) or -1 (prev file).
*/
- private getNavLinkPath(path: string, fileList: string[], direction: -1 | 1) {
- if (!path || !fileList || fileList.length === 0) {
+ private getNavLinkPath(fileList: string[], direction: -1 | 1) {
+ if (!this.path || !fileList || fileList.length === 0) {
return null;
}
- let idx = fileList.indexOf(path);
+ let idx = fileList.indexOf(this.path);
if (idx === -1) {
const file = direction > 0 ? fileList[0] : fileList[fileList.length - 1];
return {path: file};
@@ -965,43 +1456,44 @@
// Private but used in tests.
initLineOfInterestAndCursor(leftSide: boolean) {
- this.$.diffHost.lineOfInterest = this._getLineOfInterest(leftSide);
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.lineOfInterest = this.getLineOfInterest(leftSide);
this.initCursor(leftSide);
}
// Private but used in tests.
displayDiffBaseAgainstLeftToast() {
- if (!this._patchRange) return;
+ if (!this.patchRange) return;
fireAlert(
this,
- `Patchset ${this._patchRange.basePatchNum} vs ` +
- `${this._patchRange.patchNum} selected. Press v + \u2190 to view ` +
- `Base vs ${this._patchRange.basePatchNum}`
+ `Patchset ${this.patchRange.basePatchNum} vs ` +
+ `${this.patchRange.patchNum} selected. Press v + \u2190 to view ` +
+ `Base vs ${this.patchRange.basePatchNum}`
);
}
private displayDiffAgainstLatestToast(latestPatchNum?: PatchSetNum) {
- if (!this._patchRange) return;
+ if (!this.patchRange) return;
const leftPatchset =
- this._patchRange.basePatchNum === PARENT
+ this.patchRange.basePatchNum === PARENT
? 'Base'
- : `Patchset ${this._patchRange.basePatchNum}`;
+ : `Patchset ${this.patchRange.basePatchNum}`;
fireAlert(
this,
`${leftPatchset} vs
- ${this._patchRange.patchNum} selected\n. Press v + \u2191 to view
+ ${this.patchRange.patchNum} selected\n. Press v + \u2191 to view
${leftPatchset} vs Patchset ${latestPatchNum}`
);
}
private displayToasts() {
- if (!this._patchRange) return;
- if (this._patchRange.basePatchNum !== PARENT) {
+ if (!this.patchRange) return;
+ if (this.patchRange.basePatchNum !== PARENT) {
this.displayDiffBaseAgainstLeftToast();
return;
}
- const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (this._patchRange.patchNum !== latestPatchNum) {
+ const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
+ if (this.patchRange.patchNum !== latestPatchNum) {
this.displayDiffAgainstLatestToast(latestPatchNum);
return;
}
@@ -1010,36 +1502,36 @@
private initCommitRange() {
let commit: CommitId | undefined;
let baseCommit: CommitId | undefined;
- if (!this._change) return;
- if (!this._patchRange || !this._patchRange.patchNum) return;
- const revisions = this._change.revisions ?? {};
+ if (!this.change) return;
+ if (!this.patchRange || !this.patchRange.patchNum) return;
+ const revisions = this.change.revisions ?? {};
for (const [commitSha, revision] of Object.entries(revisions)) {
const patchNum = revision._number;
- if (patchNum === this._patchRange.patchNum) {
+ if (patchNum === this.patchRange.patchNum) {
commit = commitSha as CommitId;
const commitObj = revision.commit;
const parents = commitObj?.parents || [];
- if (this._patchRange.basePatchNum === PARENT && parents.length) {
+ if (this.patchRange.basePatchNum === PARENT && parents.length) {
baseCommit = parents[parents.length - 1].commit;
}
- } else if (patchNum === this._patchRange.basePatchNum) {
+ } else if (patchNum === this.patchRange.basePatchNum) {
baseCommit = commitSha as CommitId;
}
}
- this._commitRange = commit && baseCommit ? {commit, baseCommit} : undefined;
+ this.commitRange = commit && baseCommit ? {commit, baseCommit} : undefined;
}
private updateUrlToDiffUrl(lineNum?: number, leftSide?: boolean) {
- if (!this._change) return;
- if (!this._patchRange) return;
- if (!this._changeNum) return;
- if (!this._path) return;
+ if (!this.change) return;
+ if (!this.patchRange) return;
+ if (!this.changeNum) return;
+ if (!this.path) return;
const url = GerritNav.getUrlForDiffById(
- this._changeNum,
- this._change.project,
- this._path,
- this._patchRange.patchNum,
- this._patchRange.basePatchNum,
+ this.changeNum,
+ this.change.project,
+ this.path,
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum,
lineNum,
leftSide
);
@@ -1049,49 +1541,49 @@
// Private but used in tests.
initPatchRange() {
let leftSide = false;
- if (!this._change) return;
+ if (!this.change) return;
if (this.params?.view !== GerritView.DIFF) return;
if (this.params?.commentId) {
- const comment = this._changeComments?.findCommentById(
+ const comment = this.changeComments?.findCommentById(
this.params.commentId
);
if (!comment) {
fireAlert(this, 'comment not found');
- GerritNav.navigateToChange(this._change);
+ GerritNav.navigateToChange(this.change);
return;
}
this.getChangeModel().updatePath(comment.path);
- const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (!latestPatchNum) throw new Error('Missing _allPatchSets');
- this._patchRange = getPatchRangeForCommentUrl(comment, latestPatchNum);
- leftSide = isInBaseOfPatchRange(comment, this._patchRange);
+ const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
+ if (!latestPatchNum) throw new Error('Missing allPatchSets');
+ this.patchRange = getPatchRangeForCommentUrl(comment, latestPatchNum);
+ leftSide = isInBaseOfPatchRange(comment, this.patchRange);
- this._focusLineNum = comment.line;
+ this.focusLineNum = comment.line;
} else {
if (this.params.path) {
this.getChangeModel().updatePath(this.params.path);
}
if (this.params.patchNum) {
- this._patchRange = {
+ this.patchRange = {
patchNum: this.params.patchNum,
basePatchNum: this.params.basePatchNum || PARENT,
};
}
if (this.params.lineNum) {
- this._focusLineNum = this.params.lineNum;
+ this.focusLineNum = this.params.lineNum;
leftSide = !!this.params.leftSide;
}
}
- assertIsDefined(this._patchRange, '_patchRange');
+ assertIsDefined(this.patchRange, 'patchRange');
this.initLineOfInterestAndCursor(leftSide);
if (this.params?.commentId) {
// url is of type /comment/{commentId} which isn't meaningful
- this.updateUrlToDiffUrl(this._focusLineNum, leftSide);
+ this.updateUrlToDiffUrl(this.focusLineNum, leftSide);
}
- this._commentMap = this._getPaths(this._patchRange);
+ this.commentMap = this.getPaths();
}
// Private but used in tests.
@@ -1105,9 +1597,9 @@
private isSameDiffLoaded(value: AppElementDiffViewParam) {
return (
- this._patchRange?.basePatchNum === value.basePatchNum &&
- this._patchRange?.patchNum === value.patchNum &&
- this._path === value.path
+ this.patchRange?.basePatchNum === value.basePatchNum &&
+ this.patchRange?.patchNum === value.patchNum &&
+ this.path === value.path
);
}
@@ -1126,11 +1618,13 @@
}
// Private but used in tests.
- _paramsChanged(value: AppElementParams) {
- if (value.view !== GerritView.DIFF) {
+ paramsChanged() {
+ if (this.params?.view !== GerritView.DIFF) {
return;
}
+ const params = this.params;
+
// The diff view is kept in the background once created. If the user
// scrolls in the change page, the scrolling is reflected in the diff view
// as well, which means the diff is scrolled to a random position based
@@ -1140,11 +1634,11 @@
// Everything in the diff view is tied to the change. It seems better to
// force the re-creation of the diff view when the change number changes.
- const changeChanged = this._changeNum !== value.changeNum;
- if (this._changeNum !== undefined && changeChanged) {
+ const changeChanged = this.changeNum !== params.changeNum;
+ if (this.changeNum !== undefined && changeChanged) {
fireEvent(this, EventType.RECREATE_DIFF_VIEW);
return;
- } else if (this._changeNum !== undefined && this.isSameDiffLoaded(value)) {
+ } else if (this.changeNum !== undefined && this.isSameDiffLoaded(params)) {
// changeNum has not changed, so check if there are changes in patchRange
// path. If no changes then we can simply render the view as is.
this.reporting.reportInteraction('diff-view-re-rendered');
@@ -1155,59 +1649,61 @@
return;
}
- this._files = {sortedFileList: [], changeFilesByPath: {}};
+ this.files = {sortedFileList: [], changeFilesByPath: {}};
if (this.isConnected) {
this.getChangeModel().updatePath(undefined);
}
- this._patchRange = undefined;
- this._commitRange = undefined;
- this._focusLineNum = undefined;
+ this.patchRange = undefined;
+ this.commitRange = undefined;
+ this.focusLineNum = undefined;
- if (value.changeNum && value.project) {
- this.restApiService.setInProjectLookup(value.changeNum, value.project);
+ if (params.changeNum && params.project) {
+ this.restApiService.setInProjectLookup(params.changeNum, params.project);
}
- this._changeNum = value.changeNum;
+ this.changeNum = params.changeNum;
this.classList.remove('hideComments');
// When navigating away from the page, there is a possibility that the
// patch number is no longer a part of the URL (say when navigating to
// the top-level change info view) and therefore undefined in `params`.
// If route is of type /comment/<commentId>/ then no patchNum is present
- if (!value.patchNum && !value.commentLink) {
+ if (!params.patchNum && !params.commentLink) {
this.reporting.error(
- new Error(`Invalid diff view URL, no patchNum found: ${value}`)
+ new Error(`Invalid diff view URL, no patchNum found: ${this.params}`)
);
return;
}
const promises: Promise<unknown>[] = [];
- if (!this._change) {
+ if (!this.change) {
promises.push(this.untilModelLoaded());
}
promises.push(this.waitUntilCommentsLoaded());
- this.$.diffHost.cancel();
- this.$.diffHost.clearDiffContent();
- this._loading = true;
+ if (this.diffHost) {
+ this.diffHost.cancel();
+ this.diffHost.clearDiffContent();
+ }
+ this.loading = true;
return Promise.all(promises)
.then(() => {
- this._loading = false;
+ this.loading = false;
this.initPatchRange();
this.initCommitRange();
- return this.$.diffHost.reload(true);
+ return this.updateComplete.then(() => this.diffHost!.reload(true));
})
.then(() => {
this.reporting.diffViewDisplayed();
})
.then(() => {
- const fileUnchanged = this.isFileUnchanged(this._diff);
- if (fileUnchanged && value.commentLink) {
- assertIsDefined(this._change, '_change');
- assertIsDefined(this._path, '_path');
- assertIsDefined(this._patchRange, '_patchRange');
+ const fileUnchanged = this.isFileUnchanged(this.diff);
+ if (fileUnchanged && params.commentLink) {
+ assertIsDefined(this.change, 'change');
+ assertIsDefined(this.path, 'path');
+ assertIsDefined(this.patchRange, 'patchRange');
- if (this._patchRange.basePatchNum === PARENT) {
+ if (this.patchRange.basePatchNum === PARENT) {
// file is unchanged between Base vs X
// hence should not show diff between Base vs Base
return;
@@ -1216,25 +1712,25 @@
fireAlert(
this,
`File is unchanged between Patchset
- ${this._patchRange.basePatchNum} and
- ${this._patchRange.patchNum}. Showing diff of Base vs
- ${this._patchRange.basePatchNum}`
+ ${this.patchRange.basePatchNum} and
+ ${this.patchRange.patchNum}. Showing diff of Base vs
+ ${this.patchRange.basePatchNum}`
);
GerritNav.navigateToDiff(
- this._change,
- this._path,
- this._patchRange.basePatchNum as RevisionPatchSetNum,
+ this.change,
+ this.path,
+ this.patchRange.basePatchNum as RevisionPatchSetNum,
PARENT,
- this._focusLineNum
+ this.focusLineNum
);
return;
}
- if (value.commentLink) {
+ if (params.commentLink) {
this.displayToasts();
}
// If the blame was loaded for a previous file and user navigates to
// another file, then we load the blame for this file too
- if (this._isBlameLoaded) this.loadBlame();
+ if (this.isBlameLoaded) this.loadBlame();
});
}
@@ -1248,7 +1744,7 @@
* Private but used in tests.
*/
initCursor(leftSide: boolean) {
- if (this._focusLineNum === undefined) {
+ if (this.focusLineNum === undefined) {
return;
}
if (!this.cursor) return;
@@ -1257,29 +1753,30 @@
} else {
this.cursor.side = Side.RIGHT;
}
- this.cursor.initialLineNumber = this._focusLineNum;
+ this.cursor.initialLineNumber = this.focusLineNum;
}
- _getLineOfInterest(leftSide: boolean): DisplayLine | undefined {
+ // Private but used in tests.
+ getLineOfInterest(leftSide: boolean): DisplayLine | undefined {
// If there is a line number specified, pass it along to the diff so that
// it will not get collapsed.
- if (!this._focusLineNum) {
+ if (!this.focusLineNum) {
return undefined;
}
return {
- lineNum: this._focusLineNum,
+ lineNum: this.focusLineNum,
side: leftSide ? Side.LEFT : Side.RIGHT,
};
}
- _pathChanged(path: string) {
- if (path) {
- fireTitleChange(this, computeTruncatedPath(path));
+ private pathChanged() {
+ if (this.path) {
+ fireTitleChange(this, computeTruncatedPath(this.path));
}
}
- _getDiffUrl(
+ private getDiffUrl(
change?: ChangeInfo | ParsedChangeInfo,
patchRange?: PatchRange,
path?: string
@@ -1298,7 +1795,7 @@
* patch) then the patch range need not appear in the URL. Return a patch
* range object with undefined values when a range is not needed.
*/
- _getChangeUrlRange(
+ private getChangeUrlRange(
patchRange?: PatchRange,
revisions?: {[revisionId: string]: RevisionInfo | EditRevisionInfo}
) {
@@ -1321,29 +1818,29 @@
return {patchNum, basePatchNum};
}
- _getChangePath(
- change?: ChangeInfo | ParsedChangeInfo,
- patchRange?: PatchRange,
- revisions?: {[revisionId: string]: RevisionInfo | EditRevisionInfo}
- ) {
- if (!change) return '';
- if (!patchRange) return '';
+ private getChangePath() {
+ if (!this.change) return '';
+ if (!this.patchRange) return '';
- const range = this._getChangeUrlRange(patchRange, revisions);
- return GerritNav.getUrlForChange(change, {
+ const range = this.getChangeUrlRange(
+ this.patchRange,
+ this.change.revisions
+ );
+ return GerritNav.getUrlForChange(this.change, {
patchNum: range.patchNum,
basePatchNum: range.basePatchNum,
});
}
- _navigateToChange(
+ // Private but used in tests.
+ navigateToChange(
change?: ChangeInfo | ParsedChangeInfo,
patchRange?: PatchRange,
revisions?: {[revisionId: string]: RevisionInfo | EditRevisionInfo},
openReplyDialog?: boolean
) {
if (!change) return;
- const range = this._getChangeUrlRange(patchRange, revisions);
+ const range = this.getChangeUrlRange(patchRange, revisions);
GerritNav.navigateToChange(change, {
patchNum: range.patchNum,
basePatchNum: range.basePatchNum,
@@ -1351,134 +1848,111 @@
});
}
- _computeChangePath(
- change?: ChangeInfo,
- patchRangeRecord?: PolymerDeepPropertyChange<PatchRange, PatchRange>,
- revisions?: {[revisionId: string]: RevisionInfo}
- ) {
- if (!patchRangeRecord) return '';
- return this._getChangePath(change, patchRangeRecord.base, revisions);
- }
-
- _formatFilesForDropdown(
- files?: Files,
- patchRange?: PatchRange,
- changeComments?: ChangeComments
- ): DropdownItem[] {
- if (!files) return [];
- if (!patchRange) return [];
- if (!changeComments) return [];
+ // Private but used in tests
+ formatFilesForDropdown(): DropdownItem[] {
+ if (!this.files) return [];
+ if (!this.patchRange) return [];
+ if (!this.changeComments) return [];
const dropdownContent: DropdownItem[] = [];
- for (const path of files.sortedFileList) {
+ for (const path of this.files.sortedFileList) {
dropdownContent.push({
text: computeDisplayPath(path),
mobileText: computeTruncatedPath(path),
value: path,
- bottomText: changeComments.computeCommentsString(
- patchRange,
+ bottomText: this.changeComments.computeCommentsString(
+ this.patchRange,
path,
- files.changeFilesByPath[path],
+ this.files.changeFilesByPath[path],
/* includeUnmodified= */ true
),
- file: {...files.changeFilesByPath[path], __path: path},
+ file: {...this.files.changeFilesByPath[path], __path: path},
});
}
return dropdownContent;
}
- _computePrefsButtonHidden(prefs?: DiffPreferencesInfo, loggedIn?: boolean) {
- return !loggedIn || !prefs;
- }
-
- _handleFileChange(e: CustomEvent) {
- if (!this._change) return;
- if (!this._patchRange) return;
+ // Private but used in tests.
+ handleFileChange(e: CustomEvent) {
+ if (!this.change) return;
+ if (!this.patchRange) return;
// This is when it gets set initially.
const path = e.detail.value;
- if (path === this._path) {
+ if (path === this.path) {
return;
}
GerritNav.navigateToDiff(
- this._change,
+ this.change,
path,
- this._patchRange.patchNum,
- this._patchRange.basePatchNum
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum
);
}
- _handlePatchChange(e: CustomEvent) {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ // Private but used in tests.
+ handlePatchChange(e: CustomEvent) {
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
const {basePatchNum, patchNum} = e.detail;
if (
- basePatchNum === this._patchRange.basePatchNum &&
- patchNum === this._patchRange.patchNum
+ basePatchNum === this.patchRange.basePatchNum &&
+ patchNum === this.patchRange.patchNum
) {
return;
}
- GerritNav.navigateToDiff(this._change, this._path, patchNum, basePatchNum);
+ GerritNav.navigateToDiff(this.change, this.path, patchNum, basePatchNum);
}
- _handlePrefsTap(e: Event) {
+ // Private but used in tests.
+ handlePrefsTap(e: Event) {
e.preventDefault();
- this.$.diffPreferencesDialog.open();
+ assertIsDefined(this.diffPreferencesDialog, 'diffPreferencesDialog');
+ this.diffPreferencesDialog.open();
}
- _computeModeSelectHideClass(diff?: DiffInfo) {
- return !diff || diff.binary ? 'hide' : '';
- }
-
- _onLineSelected(
- _: Event,
- detail: {side: Side | CommentSide; number: number}
- ) {
+ // Private but used in tests.
+ onLineSelected(e: CustomEvent) {
// for on-comment-anchor-tap side can be PARENT/REVISIONS
// for on-line-selected side can be left/right
this.updateUrlToDiffUrl(
- detail.number,
- detail.side === Side.LEFT || detail.side === CommentSide.PARENT
+ e.detail.number,
+ e.detail.side === Side.LEFT || e.detail.side === CommentSide.PARENT
);
}
- _computeDownloadDropdownLinks(
- project?: RepoName,
- changeNum?: NumericChangeId,
- patchRange?: PatchRange,
- path?: string,
- diff?: DiffInfo
- ) {
- if (!project) return [];
- if (!changeNum) return [];
- if (!patchRange || !patchRange.patchNum) return [];
- if (!path) return [];
+ // Private but used in tests.
+ computeDownloadDropdownLinks() {
+ if (!this.change?.project) return [];
+ if (!this.changeNum) return [];
+ if (!this.patchRange?.patchNum) return [];
+ if (!this.path) return [];
const links = [
{
- url: this._computeDownloadPatchLink(
- project,
- changeNum,
- patchRange,
- path
+ url: this.computeDownloadPatchLink(
+ this.change.project,
+ this.changeNum,
+ this.patchRange,
+ this.path
),
name: 'Patch',
},
];
- if (diff && diff.meta_a) {
- let leftPath = path;
- if (diff.change_type === 'RENAMED') {
- leftPath = diff.meta_a.name;
+ if (this.diff && this.diff.meta_a) {
+ let leftPath = this.path;
+ if (this.diff.change_type === 'RENAMED') {
+ leftPath = this.diff.meta_a.name;
}
links.push({
- url: this._computeDownloadFileLink(
- project,
- changeNum,
- patchRange,
+ url: this.computeDownloadFileLink(
+ this.change.project,
+ this.changeNum,
+ this.patchRange,
leftPath,
true
),
@@ -1486,13 +1960,13 @@
});
}
- if (diff && diff.meta_b) {
+ if (this.diff && this.diff.meta_b) {
links.push({
- url: this._computeDownloadFileLink(
- project,
- changeNum,
- patchRange,
- path,
+ url: this.computeDownloadFileLink(
+ this.change.project,
+ this.changeNum,
+ this.patchRange,
+ this.path,
false
),
name: 'Right Content',
@@ -1502,7 +1976,8 @@
return links;
}
- _computeDownloadFileLink(
+ // Private but used in tests.
+ computeDownloadFileLink(
project: RepoName,
changeNum: NumericChangeId,
patchRange: PatchRange,
@@ -1529,7 +2004,8 @@
return url;
}
- _computeDownloadPatchLink(
+ // Private but used in tests.
+ computeDownloadPatchLink(
project: RepoName,
changeNum: NumericChangeId,
patchRange: PatchRange,
@@ -1540,41 +2016,14 @@
return url;
}
- @observe(
- '_changeComments',
- '_files.changeFilesByPath',
- '_path',
- '_patchRange',
- '_projectConfig'
- )
- _recomputeComments(
- changeComments?: ChangeComments,
- files?: {[path: string]: FileInfo},
- path?: string,
- patchRange?: PatchRange,
- projectConfig?: ConfigInfo
- ) {
- if (!files) return;
- if (!path) return;
- if (!patchRange) return;
- if (!projectConfig) return;
- if (!changeComments) return;
-
- const file = files[path];
- if (file && file.old_path) {
- this.$.diffHost.threads = changeComments.getThreadsBySideForFile(
- {path, basePath: file.old_path},
- patchRange
- );
- }
+ // Private but used in tests.
+ getPaths(): CommentMap {
+ if (!this.changeComments) return {};
+ return this.changeComments.getPaths(this.patchRange);
}
- _getPaths(patchRange: PatchRange): CommentMap {
- if (!this._changeComments) return {};
- return this._changeComments.getPaths(patchRange);
- }
-
- _computeCommentSkips(
+ // Private but used in tests.
+ computeCommentSkips(
commentMap?: CommentMap,
fileList?: string[],
path?: string
@@ -1608,33 +2057,24 @@
return skips;
}
- _computeContainerClass(editMode: boolean) {
- return editMode ? 'editMode' : '';
- }
-
- _computeEditMode(
- patchRangeRecord: PolymerDeepPropertyChange<PatchRange, PatchRange>
- ) {
- const patchRange = patchRangeRecord.base || {};
- return patchRange.patchNum === EDIT;
- }
-
- _computeBlameToggleLabel(loaded?: boolean, loading?: boolean) {
- return loaded && !loading ? 'Hide blame' : 'Show blame';
+ // Private but used in tests.
+ computeEditMode() {
+ return this.patchRange?.patchNum === EDIT;
}
// Private but used in tests.
loadBlame() {
- this._isBlameLoading = true;
+ this.isBlameLoading = true;
fireAlert(this, LOADING_BLAME);
- this.$.diffHost
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost
.loadBlame()
.then(() => {
- this._isBlameLoading = false;
+ this.isBlameLoading = false;
fireAlert(this, LOADED_BLAME);
})
.catch(() => {
- this._isBlameLoading = false;
+ this.isBlameLoading = false;
});
}
@@ -1642,135 +2082,123 @@
* Load and display blame information if it has not already been loaded.
* Otherwise hide it.
*/
- _toggleBlame() {
- if (this._isBlameLoaded) {
- this.$.diffHost.clearBlame();
+ private toggleBlame() {
+ assertIsDefined(this.diffHost, 'diffHost');
+ if (this.isBlameLoaded) {
+ this.diffHost.clearBlame();
return;
}
this.loadBlame();
}
- private handleToggleBlame() {
- this._toggleBlame();
- }
-
- _handleToggleHideAllCommentThreads() {
+ private handleToggleHideAllCommentThreads() {
toggleClass(this, 'hideComments');
}
private handleOpenFileList() {
- this.$.dropdown.open();
+ assertIsDefined(this.dropdown, 'dropdown');
+ this.dropdown.open();
}
// Private but used in tests.
handleDiffAgainstBase() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
- if (this._patchRange.basePatchNum === PARENT) {
+ if (this.patchRange.basePatchNum === PARENT) {
fireAlert(this, 'Base is already selected.');
return;
}
- GerritNav.navigateToDiff(
- this._change,
- this._path,
- this._patchRange.patchNum
- );
+ GerritNav.navigateToDiff(this.change, this.path, this.patchRange.patchNum);
}
// Private but used in tests.
handleDiffBaseAgainstLeft() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
- if (this._patchRange.basePatchNum === PARENT) {
+ if (this.patchRange.basePatchNum === PARENT) {
fireAlert(this, 'Left is already base.');
return;
}
GerritNav.navigateToDiff(
- this._change,
- this._path,
- this._patchRange.basePatchNum as RevisionPatchSetNum,
+ this.change,
+ this.path,
+ this.patchRange.basePatchNum as RevisionPatchSetNum,
PARENT,
this.params?.view === GerritView.DIFF && this.params?.commentLink
- ? this._focusLineNum
+ ? this.focusLineNum
: undefined
);
}
// Private but used in tests.
handleDiffAgainstLatest() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
- const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (this._patchRange.patchNum === latestPatchNum) {
+ const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
+ if (this.patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Latest is already selected.');
return;
}
GerritNav.navigateToDiff(
- this._change,
- this._path,
+ this.change,
+ this.path,
latestPatchNum,
- this._patchRange.basePatchNum
+ this.patchRange.basePatchNum
);
}
// Private but used in tests.
handleDiffRightAgainstLatest() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
- const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (this._patchRange.patchNum === latestPatchNum) {
+ const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
+ if (this.patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Right is already latest.');
return;
}
GerritNav.navigateToDiff(
- this._change,
- this._path,
+ this.change,
+ this.path,
latestPatchNum,
- this._patchRange.patchNum as BasePatchSetNum
+ this.patchRange.patchNum as BasePatchSetNum
);
}
// Private but used in tests.
handleDiffBaseAgainstLatest() {
- if (!this._change) return;
- if (!this._path) return;
- if (!this._patchRange) return;
+ if (!this.change) return;
+ if (!this.path) return;
+ if (!this.patchRange) return;
- const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
+ const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
if (
- this._patchRange.patchNum === latestPatchNum &&
- this._patchRange.basePatchNum === PARENT
+ this.patchRange.patchNum === latestPatchNum &&
+ this.patchRange.basePatchNum === PARENT
) {
fireAlert(this, 'Already diffing base against latest.');
return;
}
- GerritNav.navigateToDiff(this._change, this._path, latestPatchNum);
+ GerritNav.navigateToDiff(this.change, this.path, latestPatchNum);
}
- _computeBlameLoaderClass(isImageDiff?: boolean, path?: string) {
- return !isMagicPath(path) && !isImageDiff ? 'show' : '';
+ // Private but used in tests.
+ computeFileNum(files: DropdownItem[]) {
+ if (!this.path || !files) return undefined;
+
+ return files.findIndex(({value}) => value === this.path) + 1;
}
- _getRevisionInfo(change: ChangeInfo) {
- return new RevisionInfoObj(change);
- }
-
- _computeFileNum(file?: string, files?: DropdownItem[]) {
- if (!file || !files) return undefined;
-
- return files.findIndex(({value}) => value === file) + 1;
- }
-
- _computeFileNumClass(fileNum?: number, files?: DropdownItem[]) {
+ // Private but used in tests.
+ computeFileNumClass(fileNum?: number, files?: DropdownItem[]) {
if (files && fileNum && fileNum > 0) {
return 'show';
}
@@ -1778,7 +2206,8 @@
}
private handleToggleAllDiffContext() {
- this.$.diffHost.toggleAllContext();
+ assertIsDefined(this.diffHost, 'diffHost');
+ this.diffHost.toggleAllContext();
}
private handleNextUnreviewedFile() {
@@ -1787,59 +2216,34 @@
}
private navigateToNextFileWithCommentThread() {
- if (!this._path) return;
- if (!this._fileList) return;
- if (!this._patchRange) return;
- if (!this._change) return;
+ if (!this.path) return;
+ if (!this.files?.sortedFileList) return;
+ if (!this.patchRange) return;
+ if (!this.change) return;
const hasComment = (path: string) =>
- this._changeComments?.getCommentsForPath(path, this._patchRange!)
- ?.length ?? 0 > 0;
- const filesWithComments = this._fileList.filter(
- file => file === this._path || hasComment(file)
+ this.changeComments?.getCommentsForPath(path, this.patchRange!)?.length ??
+ 0 > 0;
+ const filesWithComments = this.files.sortedFileList.filter(
+ file => file === this.path || hasComment(file)
);
- this.navToFile(this._path, filesWithComments, 1, true);
+ this.navToFile(filesWithComments, 1, true);
}
- _handleReloadingDiffPreference() {
+ private handleReloadingDiffPreference() {
this.userModel.getDiffPreferences();
}
- _computeCanEdit(
- loggedIn?: boolean,
- editWeblinks?: GeneratedWebLink[],
- changeChangeRecord?: PolymerDeepPropertyChange<ChangeInfo, ChangeInfo>
- ) {
- if (!changeChangeRecord?.base) return false;
+ private computeCanEdit() {
return (
- loggedIn &&
- changeIsOpen(changeChangeRecord.base) &&
- (!editWeblinks || editWeblinks.length === 0)
+ !!this.change &&
+ !!this.loggedIn &&
+ changeIsOpen(this.change) &&
+ !this.computeShowEditLinks()
);
}
- _computeShowEditLinks(editWeblinks?: GeneratedWebLink[]) {
- return !!editWeblinks && editWeblinks.length > 0;
- }
-
- /**
- * Wrapper for using in the element template and computed properties
- */
- _computeAllPatchSets(change: ChangeInfo) {
- return computeAllPatchSets(change);
- }
-
- /**
- * Wrapper for using in the element template and computed properties
- */
- _computeDisplayPath(path: string) {
- return computeDisplayPath(path);
- }
-
- /**
- * Wrapper for using in the element template and computed properties
- */
- _computeTruncatedPath(path?: string) {
- return path ? computeTruncatedPath(path) : '';
+ private computeShowEditLinks() {
+ return !!this.editWeblinks && this.editWeblinks.length > 0;
}
createTitle(shortcutName: Shortcut, section: ShortcutSection) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
deleted file mode 100644
index 04d04ff..0000000
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_html.ts
+++ /dev/null
@@ -1,421 +0,0 @@
-/**
- * @license
- * Copyright 2020 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-a11y-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- :host {
- display: block;
- background-color: var(--view-background-color);
- }
- .hidden {
- display: none;
- }
- gr-patch-range-select {
- display: block;
- }
- gr-diff {
- border: none;
- }
- .stickyHeader {
- background-color: var(--view-background-color);
- position: sticky;
- top: 0;
- /* TODO(dhruvsri): This is required only because of 'position:relative' in
- <gr-diff-highlight> (which could maybe be removed??). */
- z-index: 1;
- box-shadow: var(--elevation-level-1);
- /* This is just for giving the box-shadow some space. */
- margin-bottom: 2px;
- }
- header,
- .subHeader {
- align-items: center;
- display: flex;
- justify-content: space-between;
- }
- header {
- padding: var(--spacing-s) var(--spacing-xl);
- border-bottom: 1px solid var(--border-color);
- }
- .changeNumberColon {
- color: transparent;
- }
- .headerSubject {
- margin-right: var(--spacing-m);
- font-weight: var(--font-weight-bold);
- }
- .patchRangeLeft {
- align-items: center;
- display: flex;
- }
- .navLink:not([href]) {
- color: var(--deemphasized-text-color);
- }
- .navLinks {
- align-items: center;
- display: flex;
- white-space: nowrap;
- }
- .navLink {
- padding: 0 var(--spacing-xs);
- }
- .reviewed {
- display: inline-block;
- margin: 0 var(--spacing-xs);
- vertical-align: top;
- position: relative;
- top: 8px;
- }
- .jumpToFileContainer {
- display: inline-block;
- word-break: break-all;
- }
- .mobile {
- display: none;
- }
- gr-button {
- padding: var(--spacing-s) 0;
- text-decoration: none;
- }
- .loading {
- color: var(--deemphasized-text-color);
- font-family: var(--header-font-family);
- font-size: var(--font-size-h1);
- font-weight: var(--font-weight-h1);
- line-height: var(--line-height-h1);
- height: 100%;
- padding: var(--spacing-l);
- text-align: center;
- }
- .subHeader {
- background-color: var(--background-color-secondary);
- flex-wrap: wrap;
- padding: 0 var(--spacing-l);
- }
- .prefsButton {
- text-align: right;
- }
- .editMode .hideOnEdit {
- display: none;
- }
- .blameLoader,
- .fileNum {
- display: none;
- }
- .blameLoader.show,
- .fileNum.show,
- .download,
- .preferences,
- .rightControls {
- align-items: center;
- display: flex;
- }
- .diffModeSelector,
- .editButton {
- align-items: center;
- display: flex;
- }
- .diffModeSelector span,
- .editButton span {
- margin-right: var(--spacing-xs);
- }
- .diffModeSelector.hide,
- .separator.hide {
- display: none;
- }
- .editButtona a {
- text-decoration: none;
- }
- @media screen and (max-width: 50em) {
- header {
- padding: var(--spacing-s) var(--spacing-l);
- }
- .dash {
- display: none;
- }
- .desktop {
- display: none;
- }
- .fileNav {
- align-items: flex-start;
- display: flex;
- margin: 0 var(--spacing-xs);
- }
- .fullFileName {
- display: block;
- font-style: italic;
- min-width: 50%;
- padding: 0 var(--spacing-xxs);
- text-align: center;
- width: 100%;
- word-wrap: break-word;
- }
- .reviewed {
- vertical-align: -1px;
- }
- .mobileNavLink {
- color: var(--primary-text-color);
- font-family: var(--header-font-family);
- font-size: var(--font-size-h2);
- font-weight: var(--font-weight-h2);
- line-height: var(--line-height-h2);
- text-decoration: none;
- }
- .mobileNavLink:not([href]) {
- color: var(--deemphasized-text-color);
- }
- .jumpToFileContainer {
- display: block;
- width: 100%;
- word-break: break-all;
- }
- /* prettier formatter removes semi-colons after css mixins. */
- /* prettier-ignore */
- gr-dropdown-list {
- width: 100%;
- --gr-select-style-width: 100%;
- --gr-select-style-display: block;
- --native-select-style-width: 100%;
- }
- }
- :host(.hideComments) {
- --gr-comment-thread-display: none;
- }
- </style>
- <div class$="stickyHeader [[_computeContainerClass(_editMode)]]">
- <h1 class="assistive-tech-only">
- Diff of [[_computeTruncatedPath(_path)]]
- </h1>
- <header>
- <div>
- <a
- href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]"
- >[[_changeNum]]</a
- ><!--
- --><span class="changeNumberColon">:</span>
- <span class="headerSubject">[[_change.subject]]</span>
- <input
- id="reviewed"
- class="reviewed hideOnEdit"
- type="checkbox"
- on-change="_handleReviewedChange"
- hidden$="[[!_loggedIn]]"
- hidden=""
- title="Toggle reviewed status of file"
- aria-label="file reviewed"
- /><!--
- -->
- <div class="jumpToFileContainer">
- <gr-dropdown-list
- id="dropdown"
- value="[[_path]]"
- on-value-change="_handleFileChange"
- items="[[_formattedFiles]]"
- initial-count="75"
- show-copy-for-trigger-text
- >
- </gr-dropdown-list>
- </div>
- </div>
- <div class="navLinks desktop">
- <span
- class$="fileNum [[_computeFileNumClass(_fileNum, _formattedFiles)]]"
- >
- File [[_fileNum]] of [[_formattedFiles.length]]
- <span class="separator"></span>
- </span>
- <a
- class="navLink"
- title="[[createTitle(Shortcut.PREV_FILE,
- ShortcutSection.NAVIGATION)]]"
- href$="[[_computeNavLinkURL(_change, _path, _fileList, -1)]]"
- >
- Prev</a
- >
- <span class="separator"></span>
- <a
- class="navLink"
- title="[[createTitle(Shortcut.UP_TO_CHANGE,
- ShortcutSection.NAVIGATION)]]"
- href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]"
- >
- Up</a
- >
- <span class="separator"></span>
- <a
- class="navLink"
- title="[[createTitle(Shortcut.NEXT_FILE,
- ShortcutSection.NAVIGATION)]]"
- href$="[[_computeNavLinkURL(_change, _path, _fileList, 1)]]"
- >
- Next</a
- >
- </div>
- </header>
- <div class="subHeader">
- <div class="patchRangeLeft">
- <gr-patch-range-select
- id="rangeSelect"
- change-num="[[_changeNum]]"
- patch-num="[[_patchRange.patchNum]]"
- base-patch-num="[[_patchRange.basePatchNum]]"
- files-weblinks="[[_filesWeblinks]]"
- available-patches="[[_allPatchSets]]"
- revisions="[[_change.revisions]]"
- revision-info="[[_revisionInfo]]"
- on-patch-range-change="_handlePatchChange"
- >
- </gr-patch-range-select>
- <span class="download desktop">
- <span class="separator"></span>
- <gr-dropdown
- link=""
- down-arrow=""
- items="[[_computeDownloadDropdownLinks(_change.project, _changeNum, _patchRange, _path, _diff)]]"
- horizontal-align="left"
- >
- <span class="downloadTitle"> Download </span>
- </gr-dropdown>
- </span>
- </div>
- <div class="rightControls">
- <span
- class$="blameLoader [[_computeBlameLoaderClass(_isImageDiff, _path)]]"
- >
- <gr-button
- link=""
- id="toggleBlame"
- title="[[createTitle(Shortcut.TOGGLE_BLAME, ShortcutSection.DIFFS)]]"
- disabled="[[_isBlameLoading]]"
- on-click="_toggleBlame"
- >[[_computeBlameToggleLabel(_isBlameLoaded,
- _isBlameLoading)]]</gr-button
- >
- </span>
- <template
- is="dom-if"
- if="[[_computeCanEdit(_loggedIn, _editWeblinks, _change.*)]]"
- >
- <span class="separator"></span>
- <span class="editButton">
- <gr-button
- link=""
- title="Edit current file"
- on-click="_goToEditFile"
- >edit</gr-button
- >
- </span>
- </template>
- <template is="dom-if" if="[[_computeShowEditLinks(_editWeblinks)]]">
- <span class="separator"></span>
- <template is="dom-repeat" items="[[_editWeblinks]]" as="weblink">
- <a target="_blank" href$="[[weblink.url]]">[[weblink.name]]</a>
- </template>
- </template>
- <span class="separator"></span>
- <div class$="diffModeSelector [[_computeModeSelectHideClass(_diff)]]">
- <span>Diff view:</span>
- <gr-diff-mode-selector
- id="modeSelect"
- save-on-change="[[_loggedIn]]"
- show-tooltip-below=""
- ></gr-diff-mode-selector>
- </div>
- <span
- id="diffPrefsContainer"
- hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
- hidden=""
- >
- <span class="preferences desktop">
- <gr-tooltip-content
- has-tooltip=""
- position-below=""
- title="Diff preferences"
- >
- <gr-button link="" class="prefsButton" on-click="_handlePrefsTap"
- ><iron-icon icon="gr-icons:settings"></iron-icon
- ></gr-button>
- </gr-tooltip-content>
- </span>
- </span>
- <gr-endpoint-decorator name="annotation-toggler">
- <span hidden="" id="annotation-span">
- <label for="annotation-checkbox" id="annotation-label"></label>
- <iron-input type="checkbox" disabled="">
- <input
- is="iron-input"
- type="checkbox"
- id="annotation-checkbox"
- disabled=""
- />
- </iron-input>
- </span>
- </gr-endpoint-decorator>
- </div>
- </div>
- <div class="fileNav mobile">
- <a
- class="mobileNavLink"
- href$="[[_computeNavLinkURL(_change, _path, _fileList, -1)]]"
- >
- <</a
- >
- <div class="fullFileName mobile">[[_computeDisplayPath(_path)]]</div>
- <a
- class="mobileNavLink"
- href$="[[_computeNavLinkURL(_change, _path, _fileList, 1)]]"
- >
- ></a
- >
- </div>
- </div>
- <div class="loading" hidden$="[[!_loading]]">Loading...</div>
- <h2 class="assistive-tech-only">Diff view</h2>
- <gr-diff-host
- id="diffHost"
- hidden=""
- hidden$="[[_loading]]"
- change-num="[[_changeNum]]"
- change="[[_change]]"
- commit-range="[[_commitRange]]"
- patch-range="[[_patchRange]]"
- file="[[_file]]"
- path="[[_path]]"
- project-name="[[_change.project]]"
- on-is-blame-loaded-changed="_onIsBlameLoadedChanged"
- on-comment-anchor-tap="_onLineSelected"
- on-line-selected="_onLineSelected"
- on-diff-changed="_onDiffChanged"
- on-edit-weblinks-changed="_onEditWeblinksChanged"
- on-files-weblinks-changed="_onFilesWeblinksChanged"
- on-is-image-diff-changed="_onIsImageDiffChanged"
- >
- </gr-diff-host>
- <gr-apply-fix-dialog
- id="applyFixDialog"
- change="[[_change]]"
- change-num="[[_changeNum]]"
- >
- </gr-apply-fix-dialog>
- <gr-diff-preferences-dialog
- id="diffPreferencesDialog"
- on-reload-diff-preference="_handleReloadingDiffPreference"
- >
- </gr-diff-preferences-dialog>
- <gr-overlay id="downloadOverlay">
- <gr-download-dialog
- id="downloadDialog"
- change="[[_change]]"
- patch-num="[[_patchRange.patchNum]]"
- config="[[_serverConfig.download]]"
- on-close="_handleDownloadDialogClose"
- ></gr-download-dialog>
- </gr-overlay>
-`;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
index b375219..799b11a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.ts
@@ -64,6 +64,7 @@
import {CommentMap} from '../../../utils/comment-util';
import {ParsedChangeInfo} from '../../../types/types';
import {assertIsDefined} from '../../../utils/common-util';
+import {GrDiffModeSelector} from '../../../embed/diff/gr-diff-mode-selector/gr-diff-mode-selector';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -119,22 +120,21 @@
stubRestApi('getPortedComments').returns(Promise.resolve({}));
element = basicFixture.instantiate();
- element._changeNum = 42 as NumericChangeId;
- element._path = 'some/path.txt';
- element._change = createParsedChange();
- element._diff = {...createDiff(), content: []};
+ element.changeNum = 42 as NumericChangeId;
+ element.path = 'some/path.txt';
+ element.change = createParsedChange();
+ element.diff = {...createDiff(), content: []};
getDiffRestApiStub = stubRestApi('getDiff');
- // Delayed in case a test updates element._diff.
- getDiffRestApiStub.callsFake(() => Promise.resolve(element._diff));
- element._patchRange = createPatchRange();
- element._changeComments = new ChangeComments({
+ // Delayed in case a test updates element.diff.
+ getDiffRestApiStub.callsFake(() => Promise.resolve(element.diff));
+ element.patchRange = createPatchRange();
+ element.changeComments = new ChangeComments({
'/COMMIT_MSG': [
createComment('c1', 10, 2, '/COMMIT_MSG'),
createComment('c3', 10, PARENT, '/COMMIT_MSG'),
],
});
- await flush();
- await element.$.diffHost.updateComplete;
+ await element.updateComplete;
element.getCommentsModel().setState({
comments: {},
@@ -153,10 +153,11 @@
test('params change triggers diffViewDisplayed()', () => {
const diffViewDisplayedStub = stubReporting('diffViewDisplayed');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'initPatchRange');
- sinon.stub(element, '_getFiles');
- const paramsChangedSpy = sinon.spy(element, '_paramsChanged');
+ sinon.stub(element, 'fetchFiles');
+ const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
element.params = {
view: GerritNav.View.DIFF,
changeNum: 42 as NumericChangeId,
@@ -164,8 +165,8 @@
basePatchNum: 1 as BasePatchSetNum,
path: '/COMMIT_MSG',
};
- element._path = '/COMMIT_MSG';
- element._patchRange = createPatchRange();
+ element.path = '/COMMIT_MSG';
+ element.patchRange = createPatchRange();
return paramsChangedSpy.returnValues[0]?.then(() => {
assert.isTrue(diffViewDisplayedStub.calledOnce);
});
@@ -183,10 +184,11 @@
);
getUrlStub = sinon.stub(GerritNav, 'getUrlForDiffById');
replaceStateStub = sinon.stub(history, 'replaceState');
- sinon.stub(element, '_getFiles');
+ sinon.stub(element, 'fetchFiles');
stubReporting('diffViewDisplayed');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
- paramsChangedSpy = sinon.spy(element, '_paramsChanged');
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
+ paramsChangedSpy = sinon.spy(element, 'paramsChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -218,7 +220,7 @@
path: 'abcd',
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(11),
};
@@ -226,12 +228,9 @@
assert.isTrue(
initLineOfInterestAndCursorStub.calledWithExactly(true)
);
- assert.equal(element._focusLineNum, 10);
- assert.equal(
- element._patchRange?.patchNum,
- 11 as RevisionPatchSetNum
- );
- assert.equal(element._patchRange?.basePatchNum, 2 as BasePatchSetNum);
+ assert.equal(element.focusLineNum, 10);
+ assert.equal(element.patchRange?.patchNum, 11 as RevisionPatchSetNum);
+ assert.equal(element.patchRange?.basePatchNum, 2 as BasePatchSetNum);
assert.isTrue(replaceStateStub.called);
assert.isTrue(
getUrlStub.calledWithExactly(
@@ -250,13 +249,14 @@
test('params change causes blame to load if it was set to true', () => {
// Blame loads for subsequent files if it was loaded for one file
- element._isBlameLoaded = true;
+ element.isBlameLoaded = true;
stubReporting('diffViewDisplayed');
const loadBlameStub = sinon.stub(element, 'loadBlame');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
- const paramsChangedSpy = sinon.spy(element, '_paramsChanged');
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
+ const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
sinon.stub(element, 'initPatchRange');
- sinon.stub(element, '_getFiles');
+ sinon.stub(element, 'fetchFiles');
element.params = {
view: GerritNav.View.DIFF,
changeNum: 42 as NumericChangeId,
@@ -264,10 +264,10 @@
basePatchNum: 1 as BasePatchSetNum,
path: '/COMMIT_MSG',
};
- element._path = '/COMMIT_MSG';
- element._patchRange = createPatchRange();
+ element.path = '/COMMIT_MSG';
+ element.patchRange = createPatchRange();
return paramsChangedSpy.returnValues[0]!.then(() => {
- assert.isTrue(element._isBlameLoaded);
+ assert.isTrue(element.isBlameLoaded);
assert.isTrue(loadBlameStub.calledOnce);
});
});
@@ -289,9 +289,10 @@
});
stubReporting('diffViewDisplayed');
sinon.stub(element, 'loadBlame');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'isFileUnchanged').returns(true);
- const paramsChangedSpy = sinon.spy(element, '_paramsChanged');
+ const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -306,14 +307,14 @@
commentLink: true,
commentId: 'c1' as UrlEncodedCommentId,
};
- element._change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(11),
};
return paramsChangedSpy.returnValues[0]?.then(() => {
assert.isTrue(
diffNavStub.lastCall.calledWithExactly(
- element._change!,
+ element.change!,
'/COMMIT_MSG',
2 as RevisionPatchSetNum,
PARENT,
@@ -340,9 +341,10 @@
});
stubReporting('diffViewDisplayed');
sinon.stub(element, 'loadBlame');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
sinon.stub(element, 'isFileUnchanged').returns(true);
- const paramsChangedSpy = sinon.spy(element, '_paramsChanged');
+ const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -357,7 +359,7 @@
commentLink: true,
commentId: 'c3' as UrlEncodedCommentId,
};
- element._change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(11),
};
@@ -415,9 +417,10 @@
stubReporting('diffViewDisplayed');
sinon.stub(element, 'loadBlame');
- sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
- const paramsChangedSpy = sinon.spy(element, '_paramsChanged');
- element._change = undefined;
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload').returns(Promise.resolve());
+ const paramsChangedSpy = sinon.spy(element, 'paramsChanged');
+ element.change = undefined;
element.getChangeModel().setState({
change: {
...createParsedChange(),
@@ -425,7 +428,7 @@
},
loadingStatus: LoadingStatus.LOADED,
});
- element._patchRange = {
+ element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -443,139 +446,138 @@
});
test('toggle left diff with a hotkey', () => {
- const toggleLeftDiffStub = sinon.stub(
- element.$.diffHost,
- 'toggleLeftDiff'
- );
+ assertIsDefined(element.diffHost);
+ const toggleLeftDiffStub = sinon.stub(element.diffHost, 'toggleLeftDiff');
MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'A');
assert.isTrue(toggleLeftDiffStub.calledOnce);
});
test('keyboard shortcuts', async () => {
clock = sinon.useFakeTimers();
- element._changeNum = 42 as NumericChangeId;
+ element.changeNum = 42 as NumericChangeId;
element.getBrowserModel().setScreenWidth(0);
- element._patchRange = {
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
a: createRevision(10),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
- element._loggedIn = true;
- await flush();
+ element.path = 'glados.txt';
+ element.loggedIn = true;
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(
- changeNavStub.lastCall.calledWith(element._change),
+ changeNavStub.lastCall.calledWith(element.change),
'Should navigate to /c/42/'
);
- await flush();
+ await element.updateComplete;
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(
diffNavStub.lastCall.calledWith(
- element._change,
+ element.change,
'wheatley.md',
10 as RevisionPatchSetNum,
PARENT
),
'Should navigate to /c/42/10/wheatley.md'
);
- element._path = 'wheatley.md';
- await flush();
+ element.path = 'wheatley.md';
+ await element.updateComplete;
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
diffNavStub.lastCall.calledWith(
- element._change,
+ element.change,
'glados.txt',
10 as RevisionPatchSetNum,
PARENT
),
'Should navigate to /c/42/10/glados.txt'
);
- element._path = 'glados.txt';
- await flush();
+ element.path = 'glados.txt';
+ await element.updateComplete;
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
diffNavStub.lastCall.calledWith(
- element._change,
+ element.change,
'chell.go',
10 as RevisionPatchSetNum,
PARENT
),
'Should navigate to /c/42/10/chell.go'
);
- element._path = 'chell.go';
- await flush();
+ element.path = 'chell.go';
+ await element.updateComplete;
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
- changeNavStub.lastCall.calledWith(element._change),
+ changeNavStub.lastCall.calledWith(element.change),
'Should navigate to /c/42/'
);
- await flush();
- assert.isTrue(element._loading);
+ await element.updateComplete;
+ assert.isTrue(element.loading);
+ assertIsDefined(element.diffPreferencesDialog);
const showPrefsStub = sinon
- .stub(element.$.diffPreferencesDialog, 'open')
+ .stub(element.diffPreferencesDialog, 'open')
.callsFake(() => Promise.resolve());
MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
- await flush();
+ await element.updateComplete;
assert(showPrefsStub.calledOnce);
assertIsDefined(element.cursor);
let scrollStub = sinon.stub(element.cursor, 'moveToNextChunk');
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
- await flush();
+ await element.updateComplete;
assert(scrollStub.calledOnce);
scrollStub = sinon.stub(element.cursor, 'moveToPreviousChunk');
MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
- await flush();
+ await element.updateComplete;
assert(scrollStub.calledOnce);
scrollStub = sinon.stub(element.cursor, 'moveToNextCommentThread');
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'N');
- await flush();
+ await element.updateComplete;
assert(scrollStub.calledOnce);
scrollStub = sinon.stub(element.cursor, 'moveToPreviousCommentThread');
MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'P');
- await flush();
+ await element.updateComplete;
assert(scrollStub.calledOnce);
- assertIsDefined(element.$.diffHost.diffElement);
+ assertIsDefined(element.diffHost);
+ assertIsDefined(element.diffHost.diffElement);
const computeContainerClassStub = sinon.stub(
- element.$.diffHost.diffElement,
+ element.diffHost.diffElement,
'_computeContainerClass'
);
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
- await flush();
- await element.$.diffHost.updateComplete;
- await flush();
+
+ await element.updateComplete;
assert(
computeContainerClassStub.lastCall.calledWithExactly(
true,
@@ -585,7 +587,7 @@
);
MockInteractions.pressAndReleaseKeyOn(element, 27, null, 'Escape');
- await flush();
+ await element.updateComplete;
assert(
computeContainerClassStub.lastCall.calledWithExactly(
true,
@@ -595,10 +597,11 @@
);
// Note that stubbing setReviewed means that the value of the
- // `element.$.reviewed` checkbox is not flipped.
+ // `element.reviewed` checkbox is not flipped.
const setReviewedStub = sinon.stub(element, 'setReviewed');
const handleToggleSpy = sinon.spy(element, 'handleToggleFileReviewed');
- element.$.reviewed.checked = false;
+ assertIsDefined(element.reviewed);
+ element.reviewed.checked = false;
assert.isFalse(handleToggleSpy.called);
assert.isFalse(setReviewedStub.called);
@@ -620,40 +623,40 @@
clock.restore();
});
- test('moveToNextCommentThread navigates to next file', () => {
+ test('moveToNextCommentThread navigates to next file', async () => {
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
- const diffChangeStub = sinon.stub(element, '_navigateToChange');
+ const diffChangeStub = sinon.stub(element, 'navigateToChange');
assertIsDefined(element.cursor);
sinon.stub(element.cursor, 'isAtEnd').returns(true);
- element._changeNum = 42 as NumericChangeId;
+ element.changeNum = 42 as NumericChangeId;
const comment: PathToCommentsInfoMap = {
'wheatley.md': [createComment('c2', 21, 10, 'wheatley.md')],
};
- element._changeComments = new ChangeComments(comment);
- element._patchRange = {
+ element.changeComments = new ChangeComments(comment);
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
a: createRevision(10),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
- element._loggedIn = true;
+ element.path = 'glados.txt';
+ element.loggedIn = true;
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'N');
- flush();
+ await element.updateComplete;
assert.isTrue(
diffNavStub.calledWithExactly(
- element._change,
+ element.change,
'wheatley.md',
10 as RevisionPatchSetNum,
PARENT,
@@ -661,26 +664,28 @@
)
);
- element._path = 'wheatley.md'; // navigated to next file
+ element.path = 'wheatley.md'; // navigated to next file
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'N');
- flush();
+ await element.updateComplete;
assert.isTrue(diffChangeStub.called);
});
- test('shift+x shortcut toggles all diff context', () => {
- const toggleStub = sinon.stub(element.$.diffHost, 'toggleAllContext');
+ test('shift+x shortcut toggles all diff context', async () => {
+ assertIsDefined(element.diffHost);
+ const toggleStub = sinon.stub(element.diffHost, 'toggleAllContext');
MockInteractions.pressAndReleaseKeyOn(element, 88, null, 'X');
- flush();
+ await element.updateComplete;
assert.isTrue(toggleStub.called);
});
- test('diff against base', () => {
- element._patchRange = {
+ test('diff against base', async () => {
+ element.patchRange = {
basePatchNum: 5 as BasePatchSetNum,
patchNum: 10 as RevisionPatchSetNum,
};
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstBase();
const args = diffNavStub.getCall(0).args;
@@ -688,15 +693,17 @@
assert.isNotOk(args[3]);
});
- test('diff against latest', () => {
- element._change = {
+ test('diff against latest', async () => {
+ element.path = 'foo';
+ element.change = {
...createParsedChange(),
revisions: createRevisions(12),
};
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 5 as BasePatchSetNum,
patchNum: 10 as RevisionPatchSetNum,
};
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffAgainstLatest();
const args = diffNavStub.getCall(0).args;
@@ -704,12 +711,13 @@
assert.equal(args[3], 5 as BasePatchSetNum);
});
- test('handleDiffBaseAgainstLeft', () => {
- element._change = {
+ test('handleDiffBaseAgainstLeft', async () => {
+ element.path = 'foo';
+ element.change = {
...createParsedChange(),
revisions: createRevisions(10),
};
- element._patchRange = {
+ element.patchRange = {
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -717,6 +725,7 @@
view: GerritView.DASHBOARD,
dashboard: 'id' as DashboardId,
};
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
assert(diffNavStub.called);
@@ -727,21 +736,21 @@
});
test('handleDiffBaseAgainstLeft when initially navigating to a comment', () => {
- element._change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(10),
};
- element._patchRange = {
+ element.patchRange = {
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
- sinon.stub(element, '_paramsChanged');
+ sinon.stub(element, 'paramsChanged');
element.params = {
commentLink: true,
view: GerritView.DIFF,
changeNum: 42 as NumericChangeId,
};
- element._focusLineNum = 10;
+ element.focusLineNum = 10;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLeft();
assert(diffNavStub.called);
@@ -751,15 +760,17 @@
assert.equal(args[4], 10);
});
- test('handleDiffRightAgainstLatest', () => {
- element._change = {
+ test('handleDiffRightAgainstLatest', async () => {
+ element.path = 'foo';
+ element.change = {
...createParsedChange(),
revisions: createRevisions(10),
};
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffRightAgainstLatest();
assert(diffNavStub.called);
@@ -768,15 +779,16 @@
assert.equal(args[3], 3 as BasePatchSetNum);
});
- test('handleDiffBaseAgainstLatest', () => {
- element._change = {
+ test('handleDiffBaseAgainstLatest', async () => {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(10),
};
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
+ await element.updateComplete;
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element.handleDiffBaseAgainstLatest();
assert(diffNavStub.called);
@@ -787,11 +799,11 @@
test('A fires an error event when not logged in', async () => {
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(false));
+ element.loggedIn = false;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
- await flush();
+ await element.updateComplete;
assert.isTrue(
changeNavStub.notCalled,
'The `a` keyboard shortcut ' +
@@ -801,12 +813,12 @@
});
test('A navigates to change with logged in', async () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: 5 as BasePatchSetNum,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
@@ -815,13 +827,13 @@
},
};
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ element.loggedIn = true;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
- await flush();
+ await element.updateComplete;
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 10 as RevisionPatchSetNum,
basePatchNum: 5 as BasePatchSetNum,
openReplyDialog: true,
@@ -832,12 +844,12 @@
});
test('A navigates to change with old patch number with logged in', async () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
@@ -846,13 +858,13 @@
},
};
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ element.loggedIn = true;
const loggedInErrorSpy = sinon.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
- await flush();
+ await element.updateComplete;
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
openReplyDialog: true,
@@ -863,12 +875,12 @@
});
test('keyboard shortcuts with patch range', () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: 5 as BasePatchSetNum,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
@@ -876,19 +888,19 @@
b: createRevision(5),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
+ element.path = 'glados.txt';
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 10 as RevisionPatchSetNum,
basePatchNum: 5 as BasePatchSetNum,
openReplyDialog: false,
@@ -897,10 +909,10 @@
);
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'wheatley.md',
10 as RevisionPatchSetNum,
5 as BasePatchSetNum,
@@ -908,13 +920,13 @@
),
'Should navigate to /c/42/5..10/wheatley.md'
);
- element._path = 'wheatley.md';
+ element.path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'glados.txt',
10 as RevisionPatchSetNum,
5 as BasePatchSetNum,
@@ -922,13 +934,13 @@
),
'Should navigate to /c/42/5..10/glados.txt'
);
- element._path = 'glados.txt';
+ element.path = 'glados.txt';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'chell.go',
10 as RevisionPatchSetNum,
5 as BasePatchSetNum,
@@ -936,12 +948,12 @@
),
'Should navigate to /c/42/5..10/chell.go'
);
- element._path = 'chell.go';
+ element.path = 'chell.go';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 10 as RevisionPatchSetNum,
basePatchNum: 5 as BasePatchSetNum,
openReplyDialog: false,
@@ -949,20 +961,21 @@
'Should navigate to /c/42/5..10'
);
+ assertIsDefined(element.downloadOverlay);
const downloadOverlayStub = sinon
- .stub(element.$.downloadOverlay, 'open')
+ .stub(element.downloadOverlay, 'open')
.returns(Promise.resolve());
MockInteractions.pressAndReleaseKeyOn(element, 68, null, 'd');
assert.isTrue(downloadOverlayStub.called);
});
test('keyboard shortcuts with old patch number', () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
@@ -970,19 +983,19 @@
b: createRevision(2),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
+ element.path = 'glados.txt';
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
const changeNavStub = sinon.stub(GerritNav, 'navigateToChange');
MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
openReplyDialog: false,
@@ -993,7 +1006,7 @@
MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']');
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'wheatley.md',
1 as RevisionPatchSetNum,
PARENT,
@@ -1001,12 +1014,12 @@
),
'Should navigate to /c/42/1/wheatley.md'
);
- element._path = 'wheatley.md';
+ element.path = 'wheatley.md';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'glados.txt',
1 as RevisionPatchSetNum,
PARENT,
@@ -1014,12 +1027,12 @@
),
'Should navigate to /c/42/1/glados.txt'
);
- element._path = 'glados.txt';
+ element.path = 'glados.txt';
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
diffNavStub.lastCall.calledWithExactly(
- element._change,
+ element.change,
'chell.go',
1 as RevisionPatchSetNum,
PARENT,
@@ -1027,12 +1040,12 @@
),
'Should navigate to /c/42/1/chell.go'
);
- element._path = 'chell.go';
+ element.path = 'chell.go';
changeNavStub.reset();
MockInteractions.pressAndReleaseKeyOn(element, 219, null, '[');
assert(
- changeNavStub.lastCall.calledWithExactly(element._change, {
+ changeNavStub.lastCall.calledWithExactly(element.change, {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
openReplyDialog: false,
@@ -1043,13 +1056,13 @@
});
test('edit should redirect to edit page', async () => {
- element._loggedIn = true;
- element._path = 't.txt';
- element._patchRange = {
+ element.loggedIn = true;
+ element.path = 't.txt';
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
project: 'gerrit' as RepoName,
@@ -1060,7 +1073,7 @@
},
};
const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
- await flush();
+ await element.updateComplete;
const editBtn = queryAndAssert(element, '.editButton gr-button');
assert.isTrue(!!editBtn);
MockInteractions.tap(editBtn);
@@ -1068,9 +1081,9 @@
assert.isTrue(
redirectStub.lastCall.calledWithExactly(
GerritNav.getEditUrlForDiff(
- element._change,
- element._path,
- element._patchRange.patchNum
+ element.change,
+ element.path,
+ element.patchRange.patchNum
)
)
);
@@ -1078,13 +1091,13 @@
test('edit should redirect to edit page with line number', async () => {
const lineNumber = 42;
- element._loggedIn = true;
- element._path = 't.txt';
- element._patchRange = {
+ element.loggedIn = true;
+ element.path = 't.txt';
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
project: 'gerrit' as RepoName,
@@ -1099,7 +1112,7 @@
.stub(element.cursor, 'getAddress')
.returns({number: lineNumber, leftSide: false});
const redirectStub = sinon.stub(GerritNav, 'navigateToRelativeUrl');
- await flush();
+ await element.updateComplete;
const editBtn = queryAndAssert(element, '.editButton gr-button');
assert.isTrue(!!editBtn);
MockInteractions.tap(editBtn);
@@ -1107,9 +1120,9 @@
assert.isTrue(
redirectStub.lastCall.calledWithExactly(
GerritNav.getEditUrlForDiff(
- element._change,
- element._path,
- element._patchRange.patchNum,
+ element.change,
+ element.path,
+ element.patchRange.patchNum,
lineNumber
)
)
@@ -1124,13 +1137,13 @@
changeStatus: ChangeStatus;
}) {
return new Promise(resolve => {
- element._loggedIn = loggedIn;
- element._path = 't.txt';
- element._patchRange = {
+ element.loggedIn = loggedIn;
+ element.path = 't.txt';
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
status: changeStatus,
@@ -1192,34 +1205,34 @@
});
suite('diff prefs hidden', () => {
- test('when no prefs or logged out', () => {
- element._prefs = undefined;
- element._loggedIn = false;
- flush();
- assert.isTrue(element.$.diffPrefsContainer.hidden);
+ test('when no prefs or logged out', async () => {
+ const getDiffPrefsContainer = () =>
+ query<HTMLSpanElement>(element, '#diffPrefsContainer');
+ element.prefs = undefined;
+ element.loggedIn = false;
+ await element.updateComplete;
+ assert.isNotOk(getDiffPrefsContainer());
- element._loggedIn = true;
- flush();
- assert.isTrue(element.$.diffPrefsContainer.hidden);
+ element.loggedIn = true;
+ await element.updateComplete;
+ assert.isNotOk(getDiffPrefsContainer());
- element._loggedIn = false;
- element._prefs = {...createDefaultDiffPrefs(), font_size: 12};
- flush();
- assert.isTrue(element.$.diffPrefsContainer.hidden);
+ element.loggedIn = false;
+ element.prefs = {...createDefaultDiffPrefs(), font_size: 12};
+ await element.updateComplete;
+ assert.isNotOk(getDiffPrefsContainer());
- element._loggedIn = true;
- element._prefs = {...createDefaultDiffPrefs(), font_size: 12};
- flush();
- assert.isFalse(element.$.diffPrefsContainer.hidden);
+ element.loggedIn = true;
+ element.prefs = {...createDefaultDiffPrefs(), font_size: 12};
+ await element.updateComplete;
+ assert.isOk(getDiffPrefsContainer());
});
});
test('prefsButton opens gr-diff-preferences', () => {
- const handlePrefsTapSpy = sinon.spy(element, '_handlePrefsTap');
- const overlayOpenStub = sinon.stub(
- element.$.diffPreferencesDialog,
- 'open'
- );
+ const handlePrefsTapSpy = sinon.spy(element, 'handlePrefsTap');
+ assertIsDefined(element.diffPreferencesDialog);
+ const overlayOpenStub = sinon.stub(element.diffPreferencesDialog, 'open');
const prefsButton = queryAndAssert(element, '.prefsButton');
MockInteractions.tap(prefsButton);
@@ -1229,7 +1242,7 @@
suite('url params', () => {
setup(() => {
- sinon.stub(element, '_getFiles');
+ sinon.stub(element, 'fetchFiles');
sinon
.stub(GerritNav, 'getUrlForDiff')
.callsFake((c, p, pn, bpn) => `${c._number}-${p}-${pn}-${bpn}`);
@@ -1241,23 +1254,23 @@
});
test('_formattedFiles', () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
'/COMMIT_MSG',
'/MERGE_LIST',
]);
- element._path = 'glados.txt';
+ element.path = 'glados.txt';
const expectedFormattedFiles: DropdownItem[] = [
{
text: 'chell.go',
@@ -1311,30 +1324,33 @@
},
];
- assert.deepEqual(element._formattedFiles, expectedFormattedFiles);
- assert.equal(element._formattedFiles?.[1].value, element._path);
+ const result = element.formatFilesForDropdown();
+
+ assert.deepEqual(result, expectedFormattedFiles);
+ assert.equal(result[1].value, element.path);
});
- test('prev/up/next links', () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ test('prev/up/next links', async () => {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
a: createRevision(10),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
- flush();
+ element.path = 'glados.txt';
+ await element.updateComplete;
+
const linkEls = queryAll(element, '.navLink');
assert.equal(linkEls.length, 3);
assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-PARENT');
@@ -1343,24 +1359,24 @@
linkEls[2].getAttribute('href'),
'42-wheatley.md-10-PARENT'
);
- element._path = 'wheatley.md';
- flush();
+ element.path = 'wheatley.md';
+ await element.updateComplete;
assert.equal(
linkEls[0].getAttribute('href'),
'42-glados.txt-10-PARENT'
);
assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined');
assert.equal(linkEls[2].getAttribute('href'), '42-undefined-undefined');
- element._path = 'chell.go';
- flush();
+ element.path = 'chell.go';
+ await element.updateComplete;
assert.equal(linkEls[0].getAttribute('href'), '42-undefined-undefined');
assert.equal(linkEls[1].getAttribute('href'), '42-undefined-undefined');
assert.equal(
linkEls[2].getAttribute('href'),
'42-glados.txt-10-PARENT'
);
- element._path = 'not_a_real_file';
- flush();
+ element.path = 'not_a_real_file';
+ await element.updateComplete;
assert.equal(
linkEls[0].getAttribute('href'),
'42-wheatley.md-10-PARENT'
@@ -1369,13 +1385,13 @@
assert.equal(linkEls[2].getAttribute('href'), '42-chell.go-10-PARENT');
});
- test('prev/up/next links with patch range', () => {
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ test('prev/up/next links with patch range', async () => {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: 5 as BasePatchSetNum,
patchNum: 10 as RevisionPatchSetNum,
};
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
revisions: {
@@ -1383,58 +1399,59 @@
b: createRevision(10),
},
};
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'chell.go',
'glados.txt',
'wheatley.md',
]);
- element._path = 'glados.txt';
- flush();
+ element.path = 'glados.txt';
+ await element.updateComplete;
const linkEls = queryAll(element, '.navLink');
assert.equal(linkEls.length, 3);
assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-5');
assert.equal(linkEls[1].getAttribute('href'), '42-10-5');
assert.equal(linkEls[2].getAttribute('href'), '42-wheatley.md-10-5');
- element._path = 'wheatley.md';
- flush();
+ element.path = 'wheatley.md';
+ await element.updateComplete;
assert.equal(linkEls[0].getAttribute('href'), '42-glados.txt-10-5');
assert.equal(linkEls[1].getAttribute('href'), '42-10-5');
assert.equal(linkEls[2].getAttribute('href'), '42-10-5');
- element._path = 'chell.go';
- flush();
+ element.path = 'chell.go';
+ await element.updateComplete;
assert.equal(linkEls[0].getAttribute('href'), '42-10-5');
assert.equal(linkEls[1].getAttribute('href'), '42-10-5');
assert.equal(linkEls[2].getAttribute('href'), '42-glados.txt-10-5');
});
});
- test('_handlePatchChange calls navigateToDiff correctly', () => {
+ test('handlePatchChange calls navigateToDiff correctly', async () => {
const navigateStub = sinon.stub(GerritNav, 'navigateToDiff');
- element._change = {
+ element.change = {
...createParsedChange(),
_number: 321 as NumericChangeId,
project: 'foo/bar' as RepoName,
};
- element._path = 'path/to/file.txt';
+ element.path = 'path/to/file.txt';
- element._patchRange = {
+ element.patchRange = {
basePatchNum: PARENT,
patchNum: 3 as RevisionPatchSetNum,
};
+ await element.updateComplete;
const detail = {
basePatchNum: PARENT,
patchNum: 1 as RevisionPatchSetNum,
};
- element.$.rangeSelect.dispatchEvent(
+ queryAndAssert(element, '#rangeSelect').dispatchEvent(
new CustomEvent('patch-range-change', {detail, bubbles: false})
);
assert(
navigateStub.lastCall.calledWithExactly(
- element._change,
- element._path,
+ element.change,
+ element.path,
1 as RevisionPatchSetNum,
PARENT
)
@@ -1451,8 +1468,9 @@
const setReviewedStatusStub = sinon.spy(element, 'setReviewedStatus');
- sinon.stub(element.$.diffHost, 'reload');
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload');
+ element.loggedIn = true;
const diffPreferences = {
...createDefaultDiffPrefs(),
manual_review: true,
@@ -1470,7 +1488,7 @@
view: GerritView.DIFF,
patchNum: 2 as RevisionPatchSetNum,
});
- element._patchRange = {
+ element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -1482,7 +1500,7 @@
// if prefs are updated then the reviewed status should not be set again
element.userModel.setDiffPreferences(createDefaultDiffPrefs());
- await flush();
+ await element.updateComplete;
assert.isFalse(setReviewedFileStatusStub.called);
}
);
@@ -1492,8 +1510,9 @@
.stub(element.getChangeModel(), 'setReviewedFilesStatus')
.callsFake(() => Promise.resolve());
- sinon.stub(element.$.diffHost, 'reload');
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload');
+ element.loggedIn = true;
const diffPreferences = {
...createDefaultDiffPrefs(),
manual_review: false,
@@ -1511,7 +1530,7 @@
view: GerritView.DIFF,
patchNum: 22 as RevisionPatchSetNum,
});
- element._patchRange = {
+ element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -1528,11 +1547,12 @@
reviewedFiles: [],
loadingStatus: LoadingStatus.LOADED,
});
- sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ element.loggedIn = true;
const saveReviewedStub = sinon
.stub(element.getChangeModel(), 'setReviewedFilesStatus')
.callsFake(() => Promise.resolve());
- sinon.stub(element.$.diffHost, 'reload');
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload');
element.userModel.setDiffPreferences(createDefaultDiffPrefs());
@@ -1542,7 +1562,7 @@
patchNum: 2 as RevisionPatchSetNum,
});
- element._patchRange = {
+ element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -1550,7 +1570,7 @@
await waitUntil(() => saveReviewedStub.called);
element.getChangeModel().updateStateFileReviewed('/COMMIT_MSG', true);
- await flush();
+ await element.updateComplete;
const reviewedStatusCheckBox = queryAndAssert<HTMLInputElement>(
element,
@@ -1575,7 +1595,7 @@
]);
element.getChangeModel().updateStateFileReviewed('/COMMIT_MSG', false);
- await flush();
+ await element.updateComplete;
MockInteractions.tap(reviewedStatusCheckBox);
assert.isTrue(reviewedStatusCheckBox.checked);
@@ -1588,8 +1608,12 @@
const callCount = saveReviewedStub.callCount;
- element.set('params.view', GerritNav.View.CHANGE);
- await flush();
+ element.params = {
+ view: GerritNav.View.CHANGE,
+ changeNum: 42 as NumericChangeId,
+ project: 'test' as RepoName,
+ };
+ await element.updateComplete;
// saveReviewedState observer observes params, but should not fire when
// view !== GerritNav.View.DIFF.
@@ -1602,22 +1626,23 @@
'setReviewedFilesStatus'
);
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 1 as BasePatchSetNum,
patchNum: EDIT,
};
flush();
- assert.isTrue(element._editMode);
+ assert.isTrue(element.computeEditMode());
element.setReviewed(true);
assert.isFalse(saveReviewedStub.called);
});
test('hash is determined from params', async () => {
- sinon.stub(element.$.diffHost, 'reload');
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload');
const initLineStub = sinon.stub(element, 'initLineOfInterestAndCursor');
- element._loggedIn = true;
+ element.loggedIn = true;
element.params = {
view: GerritNav.View.DIFF,
changeNum: 42 as NumericChangeId,
@@ -1626,14 +1651,16 @@
path: '/COMMIT_MSG',
};
+ await element.updateComplete;
await flush();
assert.isTrue(initLineStub.calledOnce);
});
- test('diff mode selector correctly toggles the diff', () => {
- const select = element.$.modeSelect;
- const diffDisplay = element.$.diffHost;
- element._userPrefs = {
+ test('diff mode selector correctly toggles the diff', async () => {
+ const select = queryAndAssert<GrDiffModeSelector>(element, '#modeSelect');
+ const diffDisplay = element.diffHost;
+ assertIsDefined(diffDisplay);
+ element.userPrefs = {
...createDefaultPreferences(),
diff_view: DiffViewMode.SIDE_BY_SIDE,
};
@@ -1641,9 +1668,9 @@
const userStub = stubUsers('updatePreferences');
- flush();
+ await element.updateComplete;
// The mode selected in the view state reflects the selected option.
- // assert.equal(element._userPrefs.diff_view, select.mode);
+ // assert.equal(element.userPrefs.diff_view, select.mode);
// The mode selected in the view state reflects the view rednered in the
// diff.
@@ -1659,18 +1686,18 @@
});
test('diff mode selector should be hidden for binary', async () => {
- element._diff = {
+ element.diff = {
...createDiff(),
binary: true,
content: [],
};
- await flush();
+ await element.updateComplete;
const diffModeSelector = queryAndAssert(element, '.diffModeSelector');
assert.isTrue(diffModeSelector.classList.contains('hide'));
});
- suite('_commitRange', () => {
+ suite('commitRange', () => {
const change: ParsedChangeInfo = {
...createParsedChange(),
_number: 42 as NumericChangeId,
@@ -1695,11 +1722,12 @@
},
};
setup(async () => {
- sinon.stub(element.$.diffHost, 'reload');
+ assertIsDefined(element.diffHost);
+ sinon.stub(element.diffHost, 'reload');
sinon.stub(element, 'initCursor');
- element._change = change;
- await flush();
- await element.$.diffHost.updateComplete;
+ element.change = change;
+ await element.updateComplete;
+ await element.diffHost.updateComplete;
});
test('uses the patchNum and basePatchNum ', async () => {
@@ -1710,10 +1738,10 @@
basePatchNum: 2 as BasePatchSetNum,
path: '/COMMIT_MSG',
};
- element._change = change;
+ element.change = change;
+ await element.updateComplete;
await flush();
- await element.$.diffHost.updateComplete;
- assert.deepEqual(element._commitRange, {
+ assert.deepEqual(element.commitRange, {
baseCommit: 'commit-sha-2' as CommitId,
commit: 'commit-sha-4' as CommitId,
});
@@ -1726,10 +1754,10 @@
patchNum: 5 as RevisionPatchSetNum,
path: '/COMMIT_MSG',
};
- element._change = change;
+ element.change = change;
+ await element.updateComplete;
await flush();
- await element.$.diffHost.updateComplete;
- assert.deepEqual(element._commitRange, {
+ assert.deepEqual(element.commitRange, {
commit: 'commit-sha-5' as CommitId,
baseCommit: 'sha-5-parent' as CommitId,
});
@@ -1750,40 +1778,40 @@
// Revision hash: specifies lineNum but not side.
- element._focusLineNum = 234;
+ element.focusLineNum = 234;
element.initCursor(false);
assert.equal(element.cursor.initialLineNumber, 234);
assert.equal(element.cursor.side, Side.RIGHT);
// Base hash: specifies lineNum and side.
- element._focusLineNum = 345;
+ element.focusLineNum = 345;
element.initCursor(true);
assert.equal(element.cursor.initialLineNumber, 345);
assert.equal(element.cursor.side, Side.LEFT);
// Specifies right side:
- element._focusLineNum = 123;
+ element.focusLineNum = 123;
element.initCursor(false);
assert.equal(element.cursor.initialLineNumber, 123);
assert.equal(element.cursor.side, Side.RIGHT);
});
- test('_getLineOfInterest', () => {
- assert.isUndefined(element._getLineOfInterest(false));
+ test('getLineOfInterest', () => {
+ assert.isUndefined(element.getLineOfInterest(false));
- element._focusLineNum = 12;
- let result = element._getLineOfInterest(false);
+ element.focusLineNum = 12;
+ let result = element.getLineOfInterest(false);
assert.isOk(result);
assert.equal(result!.lineNum, 12);
assert.equal(result!.side, Side.RIGHT);
- result = element._getLineOfInterest(true);
+ result = element.getLineOfInterest(true);
assert.isOk(result);
assert.equal(result!.lineNum, 12);
assert.equal(result!.side, Side.LEFT);
});
- test('_onLineSelected', () => {
+ test('onLineSelected', () => {
const getUrlStub = sinon.stub(GerritNav, 'getUrlForDiffById');
const replaceStateStub = sinon.stub(history, 'replaceState');
assertIsDefined(element.cursor);
@@ -1791,20 +1819,19 @@
.stub(element.cursor, 'getAddress')
.returns({number: 123, leftSide: false});
- element._changeNum = 321 as NumericChangeId;
- element._change = {
+ element.changeNum = 321 as NumericChangeId;
+ element.change = {
...createParsedChange(),
_number: 321 as NumericChangeId,
project: 'foo/bar' as RepoName,
};
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 3 as BasePatchSetNum,
patchNum: 5 as RevisionPatchSetNum,
};
- const e = {} as CustomEvent;
- const detail = {number: 123, side: Side.RIGHT};
+ const e = {detail: {number: 123, side: Side.RIGHT}} as CustomEvent;
- element._onLineSelected(e, detail);
+ element.onLineSelected(e);
assert.isTrue(replaceStateStub.called);
assert.isTrue(getUrlStub.called);
@@ -1819,20 +1846,19 @@
.stub(element.cursor, 'getAddress')
.returns({number: 123, leftSide: true});
- element._changeNum = 321 as NumericChangeId;
- element._change = {
+ element.changeNum = 321 as NumericChangeId;
+ element.change = {
...createParsedChange(),
_number: 321 as NumericChangeId,
project: 'foo/bar' as RepoName,
};
- element._patchRange = {
+ element.patchRange = {
basePatchNum: 3 as BasePatchSetNum,
patchNum: 5 as RevisionPatchSetNum,
};
- const e = {} as CustomEvent;
- const detail = {number: 123, side: Side.LEFT};
+ const e = {detail: {number: 123, side: Side.LEFT}} as CustomEvent;
- element._onLineSelected(e, detail);
+ element.onLineSelected(e);
assert.isTrue(replaceStateStub.called);
assert.isTrue(getUrlStub.called);
@@ -1841,7 +1867,7 @@
test('handleToggleDiffMode', () => {
const userStub = stubUsers('updatePreferences');
- element._userPrefs = {
+ element.userPrefs = {
...createDefaultPreferences(),
diff_view: DiffViewMode.SIDE_BY_SIDE,
};
@@ -1851,7 +1877,7 @@
diff_view: DiffViewMode.UNIFIED,
});
- element._userPrefs = {
+ element.userPrefs = {
...createDefaultPreferences(),
diff_view: DiffViewMode.UNIFIED,
};
@@ -1871,41 +1897,41 @@
patchNum: 3 as RevisionPatchSetNum,
path: 'abcd',
};
- await flush();
+ await element.updateComplete;
});
test('empty', () => {
- sinon.stub(element, '_getPaths').returns({});
+ sinon.stub(element, 'getPaths').returns({});
element.initPatchRange();
- assert.equal(Object.keys(element._commentMap ?? {}).length, 0);
+ assert.equal(Object.keys(element.commentMap ?? {}).length, 0);
});
test('has paths', () => {
- sinon.stub(element, '_getFiles');
- sinon.stub(element, '_getPaths').returns({
+ sinon.stub(element, 'fetchFiles');
+ sinon.stub(element, 'getPaths').returns({
'path/to/file/one.cpp': true,
'path-to/file/two.py': true,
});
- element._changeNum = 42 as NumericChangeId;
- element._patchRange = {
+ element.changeNum = 42 as NumericChangeId;
+ element.patchRange = {
basePatchNum: 3 as BasePatchSetNum,
patchNum: 5 as RevisionPatchSetNum,
};
element.initPatchRange();
- assert.deepEqual(Object.keys(element._commentMap ?? {}), [
+ assert.deepEqual(Object.keys(element.commentMap ?? {}), [
'path/to/file/one.cpp',
'path-to/file/two.py',
]);
});
});
- suite('_computeCommentSkips', () => {
+ suite('computeCommentSkips', () => {
test('empty file list', () => {
const commentMap = {
'path/one.jpg': true,
'path/three.wav': true,
};
const path = 'path/two.m4v';
- const result = element._computeCommentSkips(commentMap, [], path);
+ const result = element.computeCommentSkips(commentMap, [], path);
assert.isOk(result);
assert.isNotOk(result!.previous);
assert.isNotOk(result!.next);
@@ -1919,28 +1945,28 @@
commentMap[fileList[1]] = false;
commentMap[fileList[2]] = true;
- let result = element._computeCommentSkips(commentMap, fileList, path);
+ let result = element.computeCommentSkips(commentMap, fileList, path);
assert.isOk(result);
assert.equal(result!.previous, fileList[0]);
assert.equal(result!.next, fileList[2]);
commentMap[fileList[1]] = true;
- result = element._computeCommentSkips(commentMap, fileList, path);
+ result = element.computeCommentSkips(commentMap, fileList, path);
assert.isOk(result);
assert.equal(result!.previous, fileList[0]);
assert.equal(result!.next, fileList[2]);
path = fileList[0];
- result = element._computeCommentSkips(commentMap, fileList, path);
+ result = element.computeCommentSkips(commentMap, fileList, path);
assert.isOk(result);
assert.isNull(result!.previous);
assert.equal(result!.next, fileList[1]);
path = fileList[2];
- result = element._computeCommentSkips(commentMap, fileList, path);
+ result = element.computeCommentSkips(commentMap, fileList, path);
assert.isOk(result);
assert.equal(result!.previous, fileList[1]);
assert.isNull(result!.next);
@@ -1953,12 +1979,12 @@
setup(() => {
navToChangeStub = sinon.stub(element, 'navToChangeView');
navToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
- element._files = getFilesFromFileList([
+ element.files = getFilesFromFileList([
'path/one.jpg',
'path/two.m4v',
'path/three.wav',
]);
- element._patchRange = {
+ element.patchRange = {
patchNum: 2 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
@@ -1971,26 +1997,28 @@
assert.isFalse(navToDiffStub.called);
});
- test('no previous', () => {
+ test('no previous', async () => {
const commentMap: CommentMap = {};
- commentMap[element._fileList![0]!] = false;
- commentMap[element._fileList![1]!] = false;
- commentMap[element._fileList![2]!] = true;
- element._commentMap = commentMap;
- element._path = element._fileList![1]!;
+ commentMap[element.files.sortedFileList[0]!] = false;
+ commentMap[element.files.sortedFileList[1]!] = false;
+ commentMap[element.files.sortedFileList[2]!] = true;
+ element.commentMap = commentMap;
+ element.path = element.files.sortedFileList[1];
+ await element.updateComplete;
element.moveToPreviousFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
assert.isFalse(navToDiffStub.called);
});
- test('w/ previous', () => {
+ test('w/ previous', async () => {
const commentMap: CommentMap = {};
- commentMap[element._fileList![0]!] = true;
- commentMap[element._fileList![1]!] = false;
- commentMap[element._fileList![2]!] = true;
- element._commentMap = commentMap;
- element._path = element._fileList![1]!;
+ commentMap[element.files.sortedFileList[0]!] = true;
+ commentMap[element.files.sortedFileList[1]!] = false;
+ commentMap[element.files.sortedFileList[2]!] = true;
+ element.commentMap = commentMap;
+ element.path = element.files.sortedFileList[1];
+ await element.updateComplete;
element.moveToPreviousFileWithComment();
assert.isFalse(navToChangeStub.called);
@@ -2005,26 +2033,28 @@
assert.isFalse(navToDiffStub.called);
});
- test('no previous', () => {
+ test('no previous', async () => {
const commentMap: CommentMap = {};
- commentMap[element._fileList![0]!] = true;
- commentMap[element._fileList![1]!] = false;
- commentMap[element._fileList![2]!] = false;
- element._commentMap = commentMap;
- element._path = element._fileList![1];
+ commentMap[element.files.sortedFileList[0]!] = true;
+ commentMap[element.files.sortedFileList[1]!] = false;
+ commentMap[element.files.sortedFileList[2]!] = false;
+ element.commentMap = commentMap;
+ element.path = element.files.sortedFileList[1];
+ await element.updateComplete;
element.moveToNextFileWithComment();
assert.isTrue(navToChangeStub.calledOnce);
assert.isFalse(navToDiffStub.called);
});
- test('w/ previous', () => {
+ test('w/ previous', async () => {
const commentMap: CommentMap = {};
- commentMap[element._fileList![0]!] = true;
- commentMap[element._fileList![1]!] = false;
- commentMap[element._fileList![2]!] = true;
- element._commentMap = commentMap;
- element._path = element._fileList![1];
+ commentMap[element.files.sortedFileList[0]!] = true;
+ commentMap[element.files.sortedFileList[1]!] = false;
+ commentMap[element.files.sortedFileList[2]!] = true;
+ element.commentMap = commentMap;
+ element.path = element.files.sortedFileList[1];
+ await element.updateComplete;
element.moveToNextFileWithComment();
assert.isFalse(navToChangeStub.called);
@@ -2035,8 +2065,10 @@
});
test('_computeEditMode', () => {
- const callCompute = (range: PatchRange) =>
- element._computeEditMode({base: range, path: '', value: range});
+ const callCompute = (range: PatchRange) => {
+ element.patchRange = range;
+ return element.computeEditMode();
+ };
assert.isFalse(
callCompute({
basePatchNum: PARENT,
@@ -2051,16 +2083,18 @@
);
});
- test('_computeFileNum', () => {
+ test('computeFileNum', () => {
+ element.path = '/foo';
assert.equal(
- element._computeFileNum('/foo', [
+ element.computeFileNum([
{text: '/foo', value: '/foo'},
{text: '/bar', value: '/bar'},
]),
1
);
+ element.path = '/bar';
assert.equal(
- element._computeFileNum('/bar', [
+ element.computeFileNum([
{text: '/foo', value: '/foo'},
{text: '/bar', value: '/bar'},
]),
@@ -2068,10 +2102,10 @@
);
});
- test('_computeFileNumClass', () => {
- assert.equal(element._computeFileNumClass(0, []), '');
+ test('computeFileNumClass', () => {
+ assert.equal(element.computeFileNumClass(0, []), '');
assert.equal(
- element._computeFileNumClass(1, [
+ element.computeFileNumClass(1, [
{text: '/foo', value: '/foo'},
{text: '/bar', value: '/bar'},
]),
@@ -2079,24 +2113,27 @@
);
});
- test('f open file dropdown', () => {
- assert.isFalse(element.$.dropdown.$.dropdown.opened);
+ test('f open file dropdown', async () => {
+ assertIsDefined(element.dropdown);
+ assert.isFalse(element.dropdown.$.dropdown.opened);
MockInteractions.pressAndReleaseKeyOn(element, 70, null, 'f');
- flush();
- assert.isTrue(element.$.dropdown.$.dropdown.opened);
+ await element.updateComplete;
+ assert.isTrue(element.dropdown.$.dropdown.opened);
});
suite('blame', () => {
test('toggle blame with button', () => {
+ assertIsDefined(element.diffHost);
const toggleBlame = sinon
- .stub(element.$.diffHost, 'loadBlame')
+ .stub(element.diffHost, 'loadBlame')
.callsFake(() => Promise.resolve([]));
- MockInteractions.tap(element.$.toggleBlame);
+ MockInteractions.tap(queryAndAssert(element, '#toggleBlame'));
assert.isTrue(toggleBlame.calledOnce);
});
test('toggle blame with shortcut', () => {
+ assertIsDefined(element.diffHost);
const toggleBlame = sinon
- .stub(element.$.diffHost, 'loadBlame')
+ .stub(element.diffHost, 'loadBlame')
.callsFake(() => Promise.resolve([]));
MockInteractions.pressAndReleaseKeyOn(element, 66, null, 'b');
assert.isTrue(toggleBlame.calledOnce);
@@ -2104,19 +2141,22 @@
});
suite('editMode behavior', () => {
- setup(() => {
- element._loggedIn = true;
+ setup(async () => {
+ element.loggedIn = true;
+ await element.updateComplete;
});
- test('reviewed checkbox', () => {
- sinon.stub(element, '_handlePatchChange');
- element._patchRange = createPatchRange();
+ test('reviewed checkbox', async () => {
+ sinon.stub(element, 'handlePatchChange');
+ element.patchRange = createPatchRange();
+ await element.updateComplete;
+ assertIsDefined(element.reviewed);
// Reviewed checkbox should be shown.
- assert.isTrue(isVisible(element.$.reviewed));
- element.set('_patchRange.patchNum', EDIT);
- flush();
+ assert.isTrue(isVisible(element.reviewed));
+ element.patchRange = {...element.patchRange, patchNum: EDIT};
+ await element.updateComplete;
- assert.isFalse(isVisible(element.$.reviewed));
+ assert.isFalse(isVisible(element.reviewed));
});
});
@@ -2158,9 +2198,9 @@
moveToNextChunkStub.returns(CursorMoveResult.CLIPPED);
isAtEndStub.returns(true);
- element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
element.reviewedFiles = new Set(['file2']);
- element._path = 'file1';
+ element.path = 'file1';
nowStub.returns(5);
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
@@ -2168,11 +2208,7 @@
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
assert.isTrue(navToFileStub.called);
- assert.deepEqual(navToFileStub.lastCall.args, [
- 'file1',
- ['file1', 'file3'],
- 1,
- ]);
+ assert.deepEqual(navToFileStub.lastCall.args, [['file1', 'file3'], 1]);
});
test('does not navigate if n is tapped twice too slow', () => {
@@ -2202,9 +2238,9 @@
moveToPreviousChunkStub.returns(CursorMoveResult.CLIPPED);
isAtStartStub.returns(true);
- element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
element.reviewedFiles = new Set(['file2']);
- element._path = 'file3';
+ element.path = 'file3';
nowStub.returns(5);
MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
@@ -2212,11 +2248,7 @@
MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p');
assert.isTrue(navToFileStub.called);
- assert.deepEqual(navToFileStub.lastCall.args, [
- 'file3',
- ['file1', 'file3'],
- -1,
- ]);
+ assert.deepEqual(navToFileStub.lastCall.args, [['file1', 'file3'], -1]);
});
test('does not navigate if p is tapped twice too slow', () => {
@@ -2249,20 +2281,20 @@
});
test('shift+m navigates to next unreviewed file', () => {
- element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
element.reviewedFiles = new Set(['file1', 'file2']);
- element._path = 'file1';
+ element.path = 'file1';
const reviewedStub = sinon.stub(element, 'setReviewed');
const navStub = sinon.stub(element, 'navToFile');
MockInteractions.pressAndReleaseKeyOn(element, 77, null, 'M');
flush();
assert.isTrue(reviewedStub.lastCall.args[0]);
- assert.deepEqual(navStub.lastCall.args, ['file1', ['file1', 'file3'], 1]);
+ assert.deepEqual(navStub.lastCall.args, [['file1', 'file3'], 1]);
});
test('File change should trigger navigateToDiff once', async () => {
- element._files = getFilesFromFileList(['file1', 'file2', 'file3']);
+ element.files = getFilesFromFileList(['file1', 'file2', 'file3']);
sinon.stub(element, 'initLineOfInterestAndCursor');
const navigateToDiffStub = sinon.stub(GerritNav, 'navigateToDiff');
@@ -2274,19 +2306,19 @@
project: 'test-project' as RepoName,
path: 'file1',
};
- element._patchRange = {
+ element.patchRange = {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
};
- element._change = {
+ element.change = {
...createParsedChange(),
revisions: createRevisions(1),
};
- await flush();
+ await element.updateComplete;
assert.isTrue(navigateToDiffStub.notCalled);
// Switch to file2
- element._handleFileChange(
+ element.handleFileChange(
new CustomEvent('value-change', {detail: {value: 'file2'}})
);
assert.isTrue(navigateToDiffStub.calledOnce);
@@ -2299,7 +2331,7 @@
project: 'test-project' as RepoName,
path: 'file2',
};
- element._patchRange = {
+ element.patchRange = {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
};
@@ -2324,21 +2356,16 @@
},
];
- const base = {
+ element.change = createParsedChange();
+ element.change.project = 'test' as RepoName;
+ element.changeNum = 12 as NumericChangeId;
+ element.patchRange = {
patchNum: 1 as RevisionPatchSetNum,
basePatchNum: PARENT,
};
-
- assert.deepEqual(
- element._computeDownloadDropdownLinks(
- 'test' as RepoName,
- 12 as NumericChangeId,
- base,
- 'index.php',
- createDiff()
- ),
- downloadLinks
- );
+ element.path = 'index.php';
+ element.diff = createDiff();
+ assert.deepEqual(element.computeDownloadDropdownLinks(), downloadLinks);
});
test('_computeDownloadDropdownLinks diff returns renamed', () => {
@@ -2361,26 +2388,21 @@
diff.change_type = 'RENAMED';
diff.meta_a!.name = 'index2.php';
- const base = {
+ element.change = createParsedChange();
+ element.change.project = 'test' as RepoName;
+ element.changeNum = 12 as NumericChangeId;
+ element.patchRange = {
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 2 as BasePatchSetNum,
};
-
- assert.deepEqual(
- element._computeDownloadDropdownLinks(
- 'test' as RepoName,
- 12 as NumericChangeId,
- base,
- 'index.php',
- diff
- ),
- downloadLinks
- );
+ element.path = 'index.php';
+ element.diff = diff;
+ assert.deepEqual(element.computeDownloadDropdownLinks(), downloadLinks);
});
- test('_computeDownloadFileLink', () => {
+ test('computeDownloadFileLink', () => {
assert.equal(
- element._computeDownloadFileLink(
+ element.computeDownloadFileLink(
'test' as RepoName,
12 as NumericChangeId,
{patchNum: 1 as PatchSetNumber, basePatchNum: PARENT},
@@ -2391,7 +2413,7 @@
);
assert.equal(
- element._computeDownloadFileLink(
+ element.computeDownloadFileLink(
'test' as RepoName,
12 as NumericChangeId,
{patchNum: 1 as PatchSetNumber, basePatchNum: -2 as PatchSetNumber},
@@ -2402,7 +2424,7 @@
);
assert.equal(
- element._computeDownloadFileLink(
+ element.computeDownloadFileLink(
'test' as RepoName,
12 as NumericChangeId,
{patchNum: 3 as PatchSetNumber, basePatchNum: 2 as PatchSetNumber},
@@ -2413,7 +2435,7 @@
);
assert.equal(
- element._computeDownloadFileLink(
+ element.computeDownloadFileLink(
'test' as RepoName,
12 as NumericChangeId,
{patchNum: 3 as PatchSetNumber, basePatchNum: 2 as PatchSetNumber},
@@ -2424,9 +2446,9 @@
);
});
- test('_computeDownloadPatchLink', () => {
+ test('computeDownloadPatchLink', () => {
assert.equal(
- element._computeDownloadPatchLink(
+ element.computeDownloadPatchLink(
'test' as RepoName,
12 as NumericChangeId,
{basePatchNum: PARENT, patchNum: 1 as RevisionPatchSetNum},
@@ -2453,39 +2475,31 @@
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getReviewedFiles').returns(Promise.resolve([]));
element = basicFixture.instantiate();
- element._changeNum = 42 as NumericChangeId;
+ element.changeNum = 42 as NumericChangeId;
});
- test('_getFiles add files with comments without changes', () => {
- const patchChangeRecord = {
- base: {
- basePatchNum: 5 as BasePatchSetNum,
- patchNum: 10 as RevisionPatchSetNum,
- },
- value: {
- basePatchNum: 5 as BasePatchSetNum,
- patchNum: 10 as RevisionPatchSetNum,
- },
- path: '',
+ test('fetchFiles add files with comments without changes', () => {
+ element.patchRange = {
+ basePatchNum: 5 as BasePatchSetNum,
+ patchNum: 10 as RevisionPatchSetNum,
};
- const changeComments = {
+ element.changeComments = {
getPaths: sinon.stub().returns({
'file2.txt': {},
'file1.txt': {},
}),
} as unknown as ChangeComments;
- return element
- ._getFiles(23 as NumericChangeId, patchChangeRecord, changeComments)
- .then(() => {
- assert.deepEqual(element._files, {
- sortedFileList: ['a/b/test.c', 'file1.txt', 'file2.txt'],
- changeFilesByPath: {
- 'file1.txt': createFileInfo(),
- 'file2.txt': {status: 'U'} as FileInfo,
- 'a/b/test.c': createFileInfo(),
- },
- });
+ element.changeNum = 23 as NumericChangeId;
+ return element.fetchFiles().then(() => {
+ assert.deepEqual(element.files, {
+ sortedFileList: ['a/b/test.c', 'file1.txt', 'file2.txt'],
+ changeFilesByPath: {
+ 'file1.txt': createFileInfo(),
+ 'file2.txt': {status: 'U'} as FileInfo,
+ 'a/b/test.c': createFileInfo(),
+ },
});
+ });
});
});
});
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index a39d310..4899501 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -134,7 +134,7 @@
}
override disconnectedCallback() {
- this.storeTask?.cancel();
+ this.storeTask?.flush();
for (const cleanup of this.cleanups) cleanup();
this.cleanups = [];
super.disconnectedCallback();
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 15ff34e..ce2aff2 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -26,11 +26,10 @@
import './settings/gr-registration-dialog/gr-registration-dialog';
import './settings/gr-settings-view/gr-settings-view';
import {getBaseUrl} from '../utils/url-util';
-import {Shortcut} from '../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {GerritNav} from './core/gr-navigation/gr-navigation';
import {getAppContext} from '../services/app-context';
import {GrRouter} from './core/gr-router/gr-router';
-import {AccountDetailInfo, ServerInfo} from '../types/common';
+import {AccountDetailInfo} from '../types/common';
import {
constructServerErrorMsg,
GrErrorManager,
@@ -62,7 +61,7 @@
import {sharedStyles} from '../styles/shared-styles';
import {LitElement, PropertyValues, html, css, nothing} from 'lit';
import {customElement, property, query, state} from 'lit/decorators';
-import {ShortcutController} from './lit/shortcut-controller';
+import {Shortcut, ShortcutController} from './lit/shortcut-controller';
import {cache} from 'lit/directives/cache';
import {assertIsDefined} from '../utils/common-util';
import './gr-css-mixins';
@@ -101,8 +100,6 @@
@state() private account?: AccountDetailInfo;
- @state() private serverConfig?: ServerInfo;
-
@state() private version?: string;
@state() private showChangeListView?: boolean;
@@ -226,9 +223,6 @@
this.reporting.reportLifeCycle(LifeCycle.STARTED_AS_GUEST);
}
});
- this.restApiService.getConfig().then(config => {
- this.serverConfig = config;
- });
this.restApiService.getVersion().then(version => {
this.version = version;
this.logWelcome();
@@ -376,8 +370,7 @@
id="errorManager"
.loginUrl=${this.loginUrl}
></gr-error-manager>
- <gr-plugin-host id="plugins" .config=${this.serverConfig}>
- </gr-plugin-host>
+ <gr-plugin-host id="plugins"></gr-plugin-host>
<gr-external-style
id="externalStyleForAll"
name="app-theme"
@@ -396,7 +389,6 @@
id="search"
label="Search for changes"
.searchQuery=${(this.params as AppElementSearchParam)?.query}
- .serverConfig=${this.serverConfig}
>
</gr-smart-search>
`;
diff --git a/polygerrit-ui/app/elements/gr-app_test.ts b/polygerrit-ui/app/elements/gr-app_test.ts
index 06fce4d..2b84344 100644
--- a/polygerrit-ui/app/elements/gr-app_test.ts
+++ b/polygerrit-ui/app/elements/gr-app_test.ts
@@ -16,7 +16,6 @@
createServerInfo,
} from '../test/test-data-generators';
import {GrAppElement} from './gr-app-element';
-import {GrPluginHost} from './plugins/gr-plugin-host/gr-plugin-host';
import {GrRouter} from './core/gr-router/gr-router';
suite('gr-app tests', () => {
@@ -48,12 +47,6 @@
sinon.assert.callOrder(appStartedStub, routerStartStub);
});
- test('passes config to gr-plugin-host', () => {
- const grAppElement = queryAndAssert<GrAppElement>(grApp, '#app-element');
- const pluginHost = queryAndAssert<GrPluginHost>(grAppElement, '#plugins');
- assert.deepEqual(pluginHost.config, config);
- });
-
test('_paramsChanged sets search page', () => {
const grAppElement = queryAndAssert<GrAppElement>(grApp, '#app-element');
diff --git a/polygerrit-ui/app/elements/lit/shortcut-controller.ts b/polygerrit-ui/app/elements/lit/shortcut-controller.ts
index 9d39cd7..b315529 100644
--- a/polygerrit-ui/app/elements/lit/shortcut-controller.ts
+++ b/polygerrit-ui/app/elements/lit/shortcut-controller.ts
@@ -9,6 +9,7 @@
import {Shortcut} from '../../services/shortcuts/shortcuts-config';
import {resolve} from '../../models/dependency';
+export {Shortcut};
interface ShortcutListener {
binding: Binding;
listener: (e: KeyboardEvent) => void;
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
index 9b62743..dfdf12c 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
@@ -3,27 +3,37 @@
* Copyright 2017 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {LitElement, PropertyValues} from 'lit';
-import {customElement, property} from 'lit/decorators';
+import {LitElement} from 'lit';
+import {customElement, state} from 'lit/decorators';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {ServerInfo} from '../../../types/common';
+import {subscribe} from '../../lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
@customElement('gr-plugin-host')
export class GrPluginHost extends LitElement {
- @property({type: Object})
+ @state()
config?: ServerInfo;
- _configChanged(config: ServerInfo) {
- const jsPlugins = config.plugin?.js_resource_paths ?? [];
- const themes: string[] = config.default_theme ? [config.default_theme] : [];
- const instanceId = config.gerrit?.instance_id;
- getPluginLoader().loadPlugins([...themes, ...jsPlugins], instanceId);
- }
+ // visible for testing
+ readonly getConfigModel = resolve(this, configModelToken);
- override updated(changedProperties: PropertyValues<GrPluginHost>) {
- if (changedProperties.has('config') && this.config) {
- this._configChanged(this.config);
- }
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ config => {
+ if (!config) return;
+ const jsPlugins = config?.plugin?.js_resource_paths ?? [];
+ const themes: string[] = config?.default_theme
+ ? [config.default_theme]
+ : [];
+ const instanceId = config?.gerrit?.instance_id;
+ getPluginLoader().loadPlugins([...themes, ...jsPlugins], instanceId);
+ }
+ );
}
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
index 945af72..756221e 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
@@ -8,28 +8,32 @@
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {GrPluginHost} from './gr-plugin-host';
import {fixture, html} from '@open-wc/testing-helpers';
-import {ServerInfo} from '../../../api/rest-api';
+import {SinonStub} from 'sinon';
+import {createServerInfo} from '../../../test/test-data-generators';
suite('gr-plugin-host tests', () => {
let element: GrPluginHost;
+ let loadPluginsStub: SinonStub;
setup(async () => {
+ loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins');
element = await fixture<GrPluginHost>(html`
<gr-plugin-host></gr-plugin-host>
`);
+ await element.updateComplete;
sinon.stub(document.body, 'appendChild');
});
test('load plugins should be called', async () => {
- const loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins');
- element.config = {
+ loadPluginsStub.reset();
+ element.getConfigModel().updateServerConfig({
+ ...createServerInfo(),
plugin: {
has_avatars: false,
js_resource_paths: ['plugins/42', 'plugins/foo/bar', 'plugins/baz'],
},
- } as ServerInfo;
- await flush();
+ });
assert.isTrue(loadPluginsStub.calledOnce);
assert.isTrue(
loadPluginsStub.calledWith([
@@ -41,15 +45,15 @@
});
test('theme plugins should be loaded if enabled', async () => {
- const loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins');
- element.config = {
+ loadPluginsStub.reset();
+ element.getConfigModel().updateServerConfig({
+ ...createServerInfo(),
default_theme: 'gerrit-theme.js',
plugin: {
has_avatars: false,
js_resource_paths: ['plugins/42', 'plugins/foo/bar', 'plugins/baz'],
},
- } as ServerInfo;
- await flush();
+ });
assert.isTrue(loadPluginsStub.calledOnce);
assert.isTrue(
loadPluginsStub.calledWith([
@@ -60,4 +64,13 @@
])
);
});
+
+ test('plugins loaded with instanceId ', async () => {
+ loadPluginsStub.reset();
+ const config = createServerInfo();
+ config.gerrit.instance_id = 'test-id';
+ element.getConfigModel().updateServerConfig(config);
+ assert.isTrue(loadPluginsStub.calledOnce);
+ assert.isTrue(loadPluginsStub.calledWith([], 'test-id'));
+ });
});
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
index 6f28ddd..524f69d 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -96,7 +96,7 @@
storeTask?: DelayedTask;
override disconnectedCallback() {
- this.storeTask?.cancel();
+ this.storeTask?.flush();
super.disconnectedCallback();
}
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 3176f61..65b1778 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
@@ -167,6 +167,8 @@
<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>
+ <!-- This SVG is a copy from material.io https://fonts.google.com/icons?selected=Material+Icons:cancel -->
+ <g id="cancel"><path xmlns="http://www.w3.org/2000/svg" d="M0 0h24v24H0z" fill="none"/><path xmlns="http://www.w3.org/2000/svg" d="M12 2C6.47 2 2 6.47 2 12s4.47 10 10 10 10-4.47 10-10S17.53 2 12 2zm5 13.59L15.59 17 12 13.41 8.41 17 7 15.59 10.59 12 7 8.41 8.41 7 12 10.59 15.59 7 17 8.41 13.41 12 17 15.59z"/></g>
</defs>
</svg>
</iron-iconset-svg>`;
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index 5317832..0dff8c3 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -71,10 +71,10 @@
// TODO(TS): to avoid ts error for:
// Only public and protected methods of the base class are accessible
// via the 'super' keyword.
- // we call IronFocsablesHelper directly here
- // Currently IronFocsablesHelper is not exported from iron-focusables-helper
- // as it should so we use Polymer.IronFocsablesHelper here instead
- // (can not use the IronFocsablesHelperClass
+ // we call IronFocusablesHelper directly here
+ // Currently IronFocusablesHelper is not exported from iron-focusables-helper
+ // as it should so we use Polymer.IronFocusablesHelper here instead
+ // (can not use the IronFocusablesHelperClass
// in case different behavior due to singleton)
// once the type contains the exported member,
// should replace with:
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/embed/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index 17b3bf7..55e7453 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -26,7 +26,8 @@
*/
@property({type: Boolean}) saveOnChange = false;
- @property({type: Boolean}) showTooltipBelow = false;
+ @property({type: Boolean, attribute: 'show-tooltip-below'})
+ showTooltipBelow = false;
// visible for testing
@state() mode: DiffViewMode = DiffViewMode.SIDE_BY_SIDE;
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
deleted file mode 100644
index 7596804..0000000
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ /dev/null
@@ -1,135 +0,0 @@
-/**
- * @license
- * Copyright 2016 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import {property} from '@polymer/decorators';
-import {check, Constructor} from '../../utils/common-util';
-import {
- Shortcut,
- ShortcutSection,
- SPECIAL_SHORTCUT,
-} from '../../services/shortcuts/shortcuts-config';
-import {
- SectionView,
- ShortcutListener,
- shortcutsServiceToken,
-} from '../../services/shortcuts/shortcuts-service';
-import {DIPolymerElement, resolve} from '../../models/dependency';
-
-export {
- Shortcut,
- ShortcutSection,
- SPECIAL_SHORTCUT,
- ShortcutListener,
- SectionView,
-};
-
-export const KeyboardShortcutMixin = <T extends Constructor<DIPolymerElement>>(
- superClass: T
-) => {
- /**
- * @polymer
- * @mixinClass
- */
- class Mixin extends superClass {
- // This enables `Shortcut` to be used in the html template.
- Shortcut = Shortcut;
-
- // This enables `ShortcutSection` to be used in the html template.
- ShortcutSection = ShortcutSection;
-
- private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
-
- /** Used to disable shortcuts when the element is not visible. */
- private observer?: IntersectionObserver;
-
- /**
- * Enabling shortcuts only when the element is visible (see `observer`
- * above) is a great feature, but often what you want is for the *page* to
- * be visible, not the specific child element that registers keyboard
- * shortcuts. An example is the FileList in the ChangeView. So we allow
- * a broader observer target to be specified here, and fall back to
- * `this` as the default.
- */
- @property({type: Object})
- observerTarget: Element = this;
-
- /** Are shortcuts currently enabled? True only when element is visible. */
- private bindingsEnabled = false;
-
- override connectedCallback() {
- super.connectedCallback();
- this.createVisibilityObserver();
- this.enableBindings();
- }
-
- override disconnectedCallback() {
- this.destroyVisibilityObserver();
- this.disableBindings();
- super.disconnectedCallback();
- }
-
- /**
- * Creates an intersection observer that enables bindings when the
- * element is visible and disables them when the element is hidden.
- */
- private createVisibilityObserver() {
- if (!this.hasKeyboardShortcuts()) return;
- if (this.observer) return;
- this.observer = new IntersectionObserver(entries => {
- check(entries.length === 1, 'Expected one observer entry.');
- const isVisible = entries[0].isIntersecting;
- if (isVisible) {
- this.enableBindings();
- } else {
- this.disableBindings();
- }
- });
- this.observer.observe(this.observerTarget);
- }
-
- private destroyVisibilityObserver() {
- if (this.observer) this.observer.unobserve(this.observerTarget);
- }
-
- /**
- * Enables all the shortcuts returned by keyboardShortcuts().
- * This is a private method being called when the element becomes
- * connected or visible.
- */
- private enableBindings() {
- if (!this.hasKeyboardShortcuts()) return;
- if (this.bindingsEnabled) return;
- this.bindingsEnabled = true;
-
- this.getShortcutsService().attachHost(this, this.keyboardShortcuts());
- }
-
- /**
- * Disables all the shortcuts returned by keyboardShortcuts().
- * This is a private method being called when the element becomes
- * disconnected or invisible.
- */
- private disableBindings() {
- if (!this.bindingsEnabled) return;
- this.bindingsEnabled = false;
- this.getShortcutsService().detachHost(this);
- }
-
- private hasKeyboardShortcuts() {
- return this.keyboardShortcuts().length > 0;
- }
-
- keyboardShortcuts(): ShortcutListener[] {
- return [];
- }
- }
-
- return Mixin as T & Constructor<KeyboardShortcutMixinInterface>;
-};
-
-/** The interface corresponding to KeyboardShortcutMixin */
-export interface KeyboardShortcutMixinInterface {
- keyboardShortcuts(): ShortcutListener[];
-}
diff --git a/polygerrit-ui/app/models/config/config-model.ts b/polygerrit-ui/app/models/config/config-model.ts
index dd26182..6861348 100644
--- a/polygerrit-ui/app/models/config/config-model.ts
+++ b/polygerrit-ui/app/models/config/config-model.ts
@@ -12,6 +12,7 @@
import {select} from '../../utils/observable-util';
import {Model} from '../model';
import {define} from '../dependency';
+import {getDocsBaseUrl} from '../../utils/url-util';
export interface ConfigState {
repoConfig?: ConfigInfo;
@@ -35,6 +36,20 @@
configState => configState.serverConfig
);
+ public mergeabilityComputationBehavior$ = select(
+ this.serverConfig$,
+ serverConfig => serverConfig?.change?.mergeability_computation_behavior
+ );
+
+ public docsBaseUrl$ = select(
+ this.serverConfig$.pipe(
+ switchMap(serverConfig =>
+ from(getDocsBaseUrl(serverConfig, this.restApiService))
+ )
+ ),
+ url => url
+ );
+
private subscriptions: Subscription[];
constructor(
@@ -59,11 +74,12 @@
];
}
- updateRepoConfig(repoConfig?: ConfigInfo) {
+ private updateRepoConfig(repoConfig?: ConfigInfo) {
const current = this.subject$.getValue();
this.subject$.next({...current, repoConfig});
}
+ // visible for testing
updateServerConfig(serverConfig?: ServerInfo) {
const current = this.subject$.getValue();
this.subject$.next({...current, serverConfig});
diff --git a/polygerrit-ui/app/models/dependency.ts b/polygerrit-ui/app/models/dependency.ts
index ae9cc64..5499db2 100644
--- a/polygerrit-ui/app/models/dependency.ts
+++ b/polygerrit-ui/app/models/dependency.ts
@@ -4,7 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {ReactiveController, ReactiveControllerHost} from 'lit';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
/**
* This module provides the ability to do dependency injection in components.
@@ -180,49 +179,6 @@
}
/**
- * Because Polymer doesn't (yet) depend on ReactiveControllerHost, this adds a
- * work-around base-class to make this work for Polymer.
- */
-export class DIPolymerElement
- extends PolymerElement
- implements ReactiveControllerHost
-{
- private readonly ___controllers: ReactiveController[] = [];
-
- override connectedCallback() {
- for (const c of this.___controllers) {
- c.hostConnected?.();
- }
- super.connectedCallback();
- }
-
- override disconnectedCallback() {
- super.disconnectedCallback();
- for (const c of this.___controllers) {
- c.hostDisconnected?.();
- }
- }
-
- addController(controller: ReactiveController) {
- this.___controllers.push(controller);
-
- if (this.isConnected) controller.hostConnected?.();
- }
-
- removeController(controller: ReactiveController) {
- const idx = this.___controllers.indexOf(controller);
- if (idx < 0) return;
- this.___controllers?.splice(idx, 1);
- }
-
- requestUpdate() {}
-
- get updateComplete(): Promise<boolean> {
- return Promise.resolve(true);
- }
-}
-
-/**
* A callback for a value.
*/
type Callback<T> = (value: T) => void;
diff --git a/polygerrit-ui/app/models/dependency_test.ts b/polygerrit-ui/app/models/dependency_test.ts
index 76d5dfd..49e8580 100644
--- a/polygerrit-ui/app/models/dependency_test.ts
+++ b/polygerrit-ui/app/models/dependency_test.ts
@@ -3,10 +3,8 @@
* Copyright 2021 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {define, provide, resolve, DIPolymerElement} from './dependency';
+import {define, provide, resolve} from './dependency';
import {html, LitElement} from 'lit';
-import {customElement as polyCustomElement} from '@polymer/decorators';
-import {html as polyHtml} from '@polymer/polymer/lib/utils/html-tag';
import {customElement, property, query} from 'lit/decorators';
import '../test/common-test-setup-karma.js';
@@ -55,33 +53,11 @@
}
}
-@polyCustomElement('polymer-foo-provider')
-export class PolymerFooProviderElement extends DIPolymerElement {
- bar() {
- return this.$.bar as BarProviderElement;
- }
-
- override connectedCallback() {
- provide(this, fooToken, () => new FooImpl('foo'));
- super.connectedCallback();
- }
-
- static get template() {
- return polyHtml`<bar-provider id="bar"></bar-provider>`;
- }
-}
-
@customElement('bar-provider')
export class BarProviderElement extends LitElement {
@query('leaf-lit-element')
litChild?: LeafLitElement;
- @query('leaf-polymer-element')
- polymerChild?: LeafPolymerElement;
-
- @property({type: Boolean})
- public showLit = true;
-
override connectedCallback() {
super.connectedCallback();
provide(this, barToken, () => this.create());
@@ -94,11 +70,7 @@
}
override render() {
- if (this.showLit) {
- return html`<leaf-lit-element></leaf-lit-element>`;
- } else {
- return html`<leaf-polymer-element></leaf-polymer-element>`;
- }
+ return html`<leaf-lit-element></leaf-lit-element>`;
}
}
@@ -116,20 +88,6 @@
}
}
-@polyCustomElement('leaf-polymer-element')
-export class LeafPolymerElement extends DIPolymerElement {
- readonly barRef = resolve(this, barToken);
-
- override connectedCallback() {
- super.connectedCallback();
- assert.isDefined(this.barRef());
- }
-
- static get template() {
- return polyHtml`Hello`;
- }
-}
-
suite('Dependency', () => {
test('It instantiates', async () => {
const fixture = fixtureFromElement('lit-foo-provider');
@@ -138,13 +96,6 @@
assert.isDefined(element.bar?.litChild?.barRef());
});
- test('It instantiates in polymer', async () => {
- const fixture = fixtureFromElement('polymer-foo-provider');
- const element = fixture.instantiate();
- await element.bar().updateComplete;
- assert.isDefined(element.bar().litChild?.barRef());
- });
-
test('It works by connecting and reconnecting', async () => {
const fixture = fixtureFromElement('lit-foo-provider');
const element = fixture.instantiate();
@@ -159,29 +110,12 @@
await element.updateComplete;
assert.isDefined(element.bar?.litChild?.barRef());
});
-
- test('It works by connecting and reconnecting Polymer', async () => {
- const fixture = fixtureFromElement('lit-foo-provider');
- const element = fixture.instantiate();
- await element.updateComplete;
-
- const beta = element.bar;
- assert.isDefined(beta);
- assert.isNotNull(beta);
- assert.isDefined(element.bar?.litChild?.barRef());
-
- beta!.showLit = false;
- await element.updateComplete;
- assert.isDefined(element.bar?.polymerChild?.barRef());
- });
});
declare global {
interface HTMLElementTagNameMap {
'lit-foo-provider': LitFooProviderElement;
- 'polymer-foo-provider': PolymerFooProviderElement;
'bar-provider': BarProviderElement;
'leaf-lit-element': LeafLitElement;
- 'leaf-polymer-element': LeafPolymerElement;
}
}
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
index c27da15..7ca429b 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
@@ -26,6 +26,8 @@
import {FlagsService} from '../flags/flags';
import {define} from '../../models/dependency';
+export {Shortcut, ShortcutSection};
+
export type SectionView = Array<{binding: string[][]; text: string}>;
export interface ShortcutListener {
@@ -67,12 +69,6 @@
*/
private readonly activeShortcuts = new Set<Shortcut>();
- /**
- * Keeps track of cleanup callbacks (which remove keyboard listeners) that
- * have to be invoked when a component unregisters itself.
- */
- private readonly cleanupsPerHost = new Map<HTMLElement, (() => void)[]>();
-
/** Static map built in the constructor by iterating over the config. */
private readonly bindings = new Map<Shortcut, Binding[]>();
@@ -239,23 +235,6 @@
};
}
- /**
- * Being called by the Polymer specific KeyboardShortcutMixin.
- */
- attachHost(host: HTMLElement, shortcuts: ShortcutListener[]) {
- const cleanups: (() => void)[] = [];
- for (const s of shortcuts) {
- cleanups.push(this.addShortcutListener(s.shortcut, s.listener));
- }
- this.cleanupsPerHost.set(host, cleanups);
- }
-
- detachHost(host: HTMLElement) {
- const cleanups = this.cleanupsPerHost.get(host);
- for (const cleanup of cleanups ?? []) cleanup();
- return true;
- }
-
addListener(listener: ShortcutViewListener) {
this.listeners.add(listener);
listener(this.directoryView());
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
index 9e978a7..69c371e 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
@@ -120,9 +120,7 @@
test('active shortcuts by section', () => {
assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {});
- service.attachHost(document.createElement('div'), [
- {shortcut: Shortcut.NEXT_FILE, listener: _ => {}},
- ]);
+ service.addShortcutListener(Shortcut.NEXT_FILE, _ => {});
assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.NAVIGATION]: [
{
@@ -132,10 +130,7 @@
},
],
});
-
- service.attachHost(document.createElement('div'), [
- {shortcut: Shortcut.NEXT_LINE, listener: _ => {}},
- ]);
+ service.addShortcutListener(Shortcut.NEXT_LINE, _ => {});
assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.DIFFS]: [
{
@@ -156,10 +151,8 @@
],
});
- service.attachHost(document.createElement('div'), [
- {shortcut: Shortcut.SEARCH, listener: _ => {}},
- {shortcut: Shortcut.GO_TO_OPENED_CHANGES, listener: _ => {}},
- ]);
+ service.addShortcutListener(Shortcut.SEARCH, _ => {});
+ service.addShortcutListener(Shortcut.GO_TO_OPENED_CHANGES, _ => {});
assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.DIFFS]: [
{
@@ -196,13 +189,11 @@
test('directory view', () => {
assert.deepEqual(mapToObject(service.directoryView()), {});
- service.attachHost(document.createElement('div'), [
- {shortcut: Shortcut.GO_TO_OPENED_CHANGES, listener: _ => {}},
- {shortcut: Shortcut.NEXT_FILE, listener: _ => {}},
- {shortcut: Shortcut.NEXT_LINE, listener: _ => {}},
- {shortcut: Shortcut.SAVE_COMMENT, listener: _ => {}},
- {shortcut: Shortcut.SEARCH, listener: _ => {}},
- ]);
+ service.addShortcutListener(Shortcut.GO_TO_OPENED_CHANGES, _ => {});
+ service.addShortcutListener(Shortcut.NEXT_FILE, _ => {});
+ service.addShortcutListener(Shortcut.NEXT_LINE, _ => {});
+ service.addShortcutListener(Shortcut.SAVE_COMMENT, _ => {});
+ service.addShortcutListener(Shortcut.SEARCH, _ => {});
assert.deepEqual(mapToObject(service.directoryView()), {
[ShortcutSection.DIFFS]: [
{binding: [['j'], ['↓']], text: 'Go to next line'},
diff --git a/polygerrit-ui/app/styles/gr-submit-requirements-styles.ts b/polygerrit-ui/app/styles/gr-submit-requirements-styles.ts
index e488f71..a666bd0 100644
--- a/polygerrit-ui/app/styles/gr-submit-requirements-styles.ts
+++ b/polygerrit-ui/app/styles/gr-submit-requirements-styles.ts
@@ -14,4 +14,7 @@
iron-icon.error {
color: var(--deemphasized-text-color);
}
+ iron-icon.cancel {
+ color: var(--error-foreground);
+ }
`;
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 19dd8d7..54efa5c 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -238,6 +238,12 @@
}
return labels.filter(unique);
}
+export function iconForRequirement(requirement: SubmitRequirementResultInfo) {
+ if (isBlockingCondition(requirement)) {
+ return 'cancel';
+ }
+ return iconForStatus(requirement.status);
+}
export function iconForStatus(status: SubmitRequirementStatus) {
switch (status) {
@@ -430,3 +436,13 @@
label => !onlyInNotApplicableLabels.includes(label)
);
}
+
+export function isBlockingCondition(
+ requirement: SubmitRequirementResultInfo
+): boolean {
+ if (requirement.status !== SubmitRequirementStatus.UNSATISFIED) return false;
+
+ return !!requirement.submittability_expression_result.passing_atoms?.some(
+ atom => atom.match(/^label[0-9]*:[\w-]+=MIN$/)
+ );
+}
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 3f3aa1a..faf7e00 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -22,6 +22,7 @@
computeOrderedLabelValues,
mergeLabelInfoMaps,
getApplicableLabels,
+ isBlockingCondition,
} from './label-util';
import {
AccountId,
@@ -44,6 +45,7 @@
SubmitRequirementResultInfo,
SubmitRequirementStatus,
LabelNameToInfoMap,
+ SubmitRequirementExpressionInfoStatus,
} from '../api/rest-api';
const VALUES_0 = {
@@ -762,4 +764,47 @@
assert.deepEqual(getApplicableLabels(change), [label]);
});
});
+
+ suite('isBlockingCondition', () => {
+ test('true', () => {
+ const requirement: SubmitRequirementResultInfo = {
+ name: 'Code-Review',
+ description:
+ "At least one maximum vote for label 'Code-Review' is required",
+ status: SubmitRequirementStatus.UNSATISFIED,
+ is_legacy: false,
+ submittability_expression_result: {
+ expression:
+ 'label:Code-Review=MAX,user=non_uploader AND -label:Code-Review=MIN',
+ fulfilled: false,
+ status: SubmitRequirementExpressionInfoStatus.FAIL,
+ passing_atoms: ['label:Code-Review=MIN'],
+ failing_atoms: ['label:Code-Review=MAX,user=non_uploader'],
+ },
+ };
+ assert.isTrue(isBlockingCondition(requirement));
+ });
+
+ test('false', () => {
+ const requirement: SubmitRequirementResultInfo = {
+ name: 'Code-Review',
+ description:
+ "At least one maximum vote for label 'Code-Review' is required",
+ status: SubmitRequirementStatus.UNSATISFIED,
+ is_legacy: false,
+ submittability_expression_result: {
+ expression:
+ 'label:Code-Review=MAX,user=non_uploader AND -label:Code-Review=MIN',
+ fulfilled: false,
+ status: SubmitRequirementExpressionInfoStatus.FAIL,
+ passing_atoms: [],
+ failing_atoms: [
+ 'label:Code-Review=MAX,user=non_uploader',
+ 'label:Code-Review=MIN',
+ ],
+ },
+ };
+ assert.isFalse(isBlockingCondition(requirement));
+ });
+ });
});
