From 6c9d862e1030fbd6cb4e2dc56025289a852d5371 Mon Sep 17 00:00:00 2001 From: Andrew Bonventre Date: Fri, 3 Jun 2016 13:06:56 +0000 Subject: [PATCH] Revert "Make gr-builder a Polymer component" This reverts commit 56689af0f92ce13f90ff9369544c5f9cc0412f09. Reason for revert: This change broke adding draft comments in the diff view. Change-Id: Icfbd3eb4e24cce3e1690e7eaf12e14e5705c7e3e --- .../diff/gr-diff-builder/gr-diff-builder.html | 127 ------------------ .../gr-diff-builder-image.js | 0 .../gr-diff-builder-side-by-side.js | 0 .../gr-diff-builder-unified.js | 0 .../gr-diff-builder.js | 0 .../gr-diff-builder_test.html | 4 +- .../app/elements/diff/gr-diff/gr-diff.html | 14 +- .../app/elements/diff/gr-diff/gr-diff.js | 57 +++++++- .../elements/diff/gr-diff/gr-diff_test.html | 28 +++- 9 files changed, 87 insertions(+), 143 deletions(-) delete mode 100644 polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html rename polygerrit-ui/app/elements/diff/{gr-diff-builder => gr-diff}/gr-diff-builder-image.js (100%) rename polygerrit-ui/app/elements/diff/{gr-diff-builder => gr-diff}/gr-diff-builder-side-by-side.js (100%) rename polygerrit-ui/app/elements/diff/{gr-diff-builder => gr-diff}/gr-diff-builder-unified.js (100%) rename polygerrit-ui/app/elements/diff/{gr-diff-builder => gr-diff}/gr-diff-builder.js (100%) rename polygerrit-ui/app/elements/diff/{gr-diff-builder => gr-diff}/gr-diff-builder_test.html (99%) diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html deleted file mode 100644 index 76cb84402f..0000000000 --- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html +++ /dev/null @@ -1,127 +0,0 @@ - - - - - - - - - - - - - diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js similarity index 100% rename from polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js rename to polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-image.js diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js similarity index 100% rename from polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js rename to polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js similarity index 100% rename from polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js rename to polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js similarity index 100% rename from polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js rename to polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html similarity index 99% rename from polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html rename to polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html index c47e05d33b..5905bd991d 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html @@ -19,8 +19,8 @@ limitations under the License. gr-diff-builder - - + + + + + + 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 ea64581a7d..0056808557 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js @@ -27,6 +27,12 @@ Polymer({ is: 'gr-diff', + /** + * Fired when the diff is rendered. + * + * @event render + */ + properties: { changeNum: String, patchRange: Object, @@ -53,6 +59,7 @@ value: DiffViewMode.SIDE_BY_SIDE, }, _diff: Object, + _diffBuilder: Object, _selectionSide: { type: String, observer: '_selectionSideChanged', @@ -188,7 +195,7 @@ var el = Polymer.dom(e).rootTarget; if (el.classList.contains('showContext')) { - this.$.diffBuilder.showContext(e.detail.groups, e.detail.section); + this._showContext(e.detail.groups, e.detail.section); } else if (el.classList.contains('lineNum')) { this.addDraftAtLine(el); } @@ -216,8 +223,8 @@ patchNum = this.patchRange.basePatchNum; } } - threadEl = this.$.diffBuilder.createCommentThread( - this.changeNum, patchNum, this.path, side, this.projectConfig); + threadEl = this._builder.createCommentThread(this.changeNum, patchNum, + this.path, side, this.projectConfig); contentEl.appendChild(threadEl); } threadEl.addDraft(opt_lineNum); @@ -356,6 +363,25 @@ return text; }, + _showContext: function(newGroups, sectionEl) { + var groups = this._builder._groups; + // TODO(viktard): Polyfill findIndex for IE10. + var contextIndex = groups.findIndex(function(group) { + return group.element == sectionEl; + }); + + groups.splice.apply(groups, [contextIndex, 1].concat(newGroups)); + + newGroups.forEach(function(newGroup) { + this._builder.emitGroup(newGroup, sectionEl); + }.bind(this)); + sectionEl.parentNode.removeChild(sectionEl); + + this.async(function() { + this.fire('render', null, {bubbles: false}); + }.bind(this), 1); + }, + _prefsChanged: function(prefsChangeRecord) { var prefs = prefsChangeRecord.base; this.customStyle['--content-width'] = prefs.line_length + 'ch'; @@ -367,7 +393,17 @@ }, _render: function() { - this.$.diffBuilder.render(this._diff, this._comments, this.prefs); + this._builder = + this._getDiffBuilder(this._diff, this._comments, this.prefs); + this._renderDiff(); + }, + + _renderDiff: function() { + this._clearDiffContent(); + this._builder.emitDiff(); + this.async(function() { + this.fire('render', null, {bubbles: false}); + }, 1); }, _clearDiffContent: function() { @@ -472,6 +508,19 @@ this.changeNum, this._diff, this.patchRange); }, + _getDiffBuilder: function(diff, comments, prefs) { + if (this.isImageDiff) { + return new GrDiffBuilderImage(diff, comments, prefs, this.$.diffTable, + this._baseImage, this._revisionImage); + } else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) { + return new GrDiffBuilderSideBySide(diff, comments, prefs, + this.$.diffTable); + } else if (this.viewMode === DiffViewMode.UNIFIED) { + return new GrDiffBuilderUnified(diff, comments, prefs, + this.$.diffTable); + } + throw Error('Unsupported diff view mode: ' + this.viewMode); + }, _projectConfigChanged: function(projectConfig) { var threadEls = this._getCommentThreads(); 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 613c7fa66b..aec32b6e90 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 @@ -226,7 +226,8 @@ limitations under the License. var rendered = function() { // Recognizes that it should be an image diff. assert.isTrue(element.isImageDiff); - assert.instanceOf(element.$.diffBuilder._builder, GrDiffBuilderImage); + assert.instanceOf(element._getDiffBuilder(element._diff, + element._comments, element.prefs), GrDiffBuilderImage); // Left image rendered with the parent commit's version of the file. var leftInmage = element.$.diffTable.querySelector('td.left img'); @@ -396,5 +397,30 @@ limitations under the License. }); }); }); + + suite('renderDiff', function() { + setup(function(done) { + sinon.stub(element, 'fire'); + element._builder = { + emitDiff: sinon.stub(), + }; + element._renderDiff(); + flush(function() { + done(); + }); + }); + + teardown(function() { + element.fire.restore(); + }); + + test('fires render', function() { + assert(element.fire.calledWithExactly( + 'render', null, {bubbles: false})); + }); + test('calls emitDiff on builder', function() { + assert(element._builder.emitDiff.calledOnce); + }); + }); });