Move the key positions calculation up from gr-diff-builder to gr-diff
This change removes the comment model dependency in gr-diff-builder,
allowing an easier reuse of the component.
A follow up change will remove the comment model dependency in gr-diff
by replacing it with threadEl model.
Change-Id: I2b8c3394a5aaa5b8880af24a057008eb2692b245
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 862db10..e587953 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -293,8 +293,7 @@
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
- revision-image="[[revisionImage]]"
- line-of-interest="[[lineOfInterest]]">
+ revision-image="[[revisionImage]]">
<slot></slot>
<table
id="diffTable"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index f87e46f..d5f92f1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -35,6 +35,18 @@
RIGHT: 'right',
};
+ const Defs = {};
+
+ /**
+ * Special line number which should not be collapsed into a shared region.
+ *
+ * @typedef {{
+ * number: number,
+ * leftSide: boolean
+ * }}
+ */
+ Defs.LineOfInterest;
+
const LARGE_DIFF_THRESHOLD_LINES = 10000;
const FULL_CONTEXT = -1;
const LIMITED_CONTEXT = 10;
@@ -121,13 +133,7 @@
observer: '_viewModeObserver',
},
- /**
- * Special line number which should not be collapsed into a shared region.
- * @type {{
- * number: number,
- * leftSide: {boolean}
- * }|null}
- */
+ /** @type ?Defs.LineOfInterest */
lineOfInterest: Object,
loading: {
@@ -639,7 +645,9 @@
}
this._showWarning = false;
- this.$.diffBuilder.render(this.comments, this._getBypassPrefs());
+ const keyLocations = this._getKeyLocations(this.comments,
+ this.lineOfInterest);
+ this.$.diffBuilder.render(keyLocations, this._getBypassPrefs());
},
_handleRenderContent() {
@@ -676,6 +684,39 @@
},
/**
+ * Returns the key locations based on the comments and line of interests,
+ * where lines should not be collapsed.
+ *
+ * @param {!Object} comments
+ * @param {Defs.LineOfInterest|null} lineOfInterest
+ *
+ * @return {{left: Object<(string|number), boolean>,
+ * right: Object<(string|number), boolean>}}
+ */
+ _getKeyLocations(comments, lineOfInterest) {
+ const result = {
+ left: {},
+ right: {},
+ };
+ for (const side in comments) {
+ if (side !== GrDiffBuilder.Side.LEFT &&
+ side !== GrDiffBuilder.Side.RIGHT) {
+ continue;
+ }
+ for (const c of comments[side]) {
+ result[side][c.line || GrDiffLine.FILE] = true;
+ }
+ }
+
+ if (lineOfInterest) {
+ const side = lineOfInterest.leftSide ? 'left' : 'right';
+ result[side][lineOfInterest.number] = true;
+ }
+
+ return result;
+ },
+
+ /**
* Get the preferences object including the safety bypass context (if any).
*/
_getBypassPrefs() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 62284ad..c6ef806 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -1127,6 +1127,33 @@
assert.equal(element._computeNewlineWarningClass('foo', false), shown);
});
});
+
+ test('_getKeyLocations', () => {
+ assert.deepEqual(element._getKeyLocations({left: [], right: []}, null),
+ {left: {}, right: {}});
+ const comments = {
+ left: [{line: 123}, {}],
+ right: [{line: 456}],
+ };
+ assert.deepEqual(element._getKeyLocations(comments, null), {
+ left: {FILE: true, 123: true},
+ right: {456: true},
+ });
+
+ const lineOfInterest = {number: 789, leftSide: true};
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true, 789: true},
+ right: {456: true},
+ });
+
+ delete lineOfInterest.leftSide;
+ assert.deepEqual(
+ element._getKeyLocations(comments, lineOfInterest), {
+ left: {FILE: true, 123: true},
+ right: {456: true, 789: true},
+ });
+ });
});
a11ySuite('basic');
