Move gr-diff controls into gr-diff-view This will allow for an easier time in implementing expanding inline diffs by having gr-diff only responsible for rendering the diff itself and not other controls. Change-Id: I254ad5900091c731e2197590d6043c103216785e
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html index 5308b11..3139e89 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -18,8 +18,11 @@ <link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html"> <link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html"> <link rel="import" href="../../shared/gr-button/gr-button.html"> +<link rel="import" href="../../shared/gr-overlay/gr-overlay.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> <link rel="import" href="../gr-diff/gr-diff.html"> +<link rel="import" href="../gr-diff-preferences/gr-diff-preferences.html"> +<link rel="import" href="../gr-patch-range-select/gr-patch-range-select.html"> <dom-module id="gr-diff-view"> <template> @@ -88,6 +91,18 @@ padding: .3em 0; text-decoration: none; } + .loading { + padding: 0 var(--default-horizontal-margin) 1em; + color: #666; + } + .header { + display: flex; + justify-content: space-between; + margin: 0 var(--default-horizontal-margin) .75em; + } + .prefsButton { + text-align: right; + } @media screen and (max-width: 50em) { .dash { display: none; @@ -144,14 +159,36 @@ </select> </div> </h3> - <gr-diff id="diff" - change-num="[[_changeNum]]" - patch-range="[[_patchRange]]" - path="[[_path]]" - project-config="[[_projectConfig]]" - available-patches="[[_computeAvailablePatches(_change.revisions)]]" - on-render="_handleDiffRender"> - </gr-diff> + <div class="loading" hidden$="[[!_loading]]">Loading...</div> + <div hidden$="[[_loading]]" hidden> + <div class="header"> + <gr-patch-range-select + path="[[_path]]" + change-num="[[_changeNum]]" + patch-range="[[_patchRange]]" + available-patches="[[_computeAvailablePatches(_change.revisions)]]"> + </gr-patch-range-select> + <gr-button link + class="prefsButton" + on-tap="_handlePrefsTap" + hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]" + hidden>Diff View Preferences</gr-button> + </div> + <gr-overlay id="prefsOverlay" with-backdrop> + <gr-diff-preferences + prefs="{{_prefs}}" + on-save="_handlePrefsSave" + on-cancel="_handlePrefsCancel"></gr-diff-preferences> + </gr-overlay> + <gr-diff id="diff" + change-num="[[_changeNum]]" + patch-range="[[_patchRange]]" + path="[[_path]]" + prefs="[[_prefs]]" + project-config="[[_projectConfig]]" + on-render="_handleDiffRender"> + </gr-diff> + </div> <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> </template> <script src="gr-diff-view.js"></script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 12d7f69..e18d329 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -59,6 +59,11 @@ type: Boolean, value: false, }, + _loading: { + type: Boolean, + value: true, + }, + _prefs: Object, }, behaviors: [ @@ -115,6 +120,32 @@ }.bind(this)); }, + _getDiffPreferences: function() { + return this._getLoggedIn().then(function(loggedIn) { + if (!loggedIn) { + // These defaults should match the defaults in + // gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java + // NOTE: There are some settings that don't apply to PolyGerrit + // (Render mode being at least one of them). + return Promise.resolve({ + auto_hide_diff_table_header: true, + context: 10, + cursor_blink_rate: 0, + ignore_whitespace: 'IGNORE_NONE', + intraline_difference: true, + line_length: 100, + show_line_endings: true, + show_tabs: true, + show_whitespace_errors: true, + syntax_highlighting: true, + tab_size: 8, + theme: 'DEFAULT', + }); + } + return this.$.restAPI.getDiffPreferences(); + }.bind(this)); + }, + _handleReviewedChange: function(e) { this._setReviewed(Polymer.dom(e).rootTarget.checked); }, @@ -208,6 +239,8 @@ _paramsChanged: function(value) { if (value.view != this.tagName.toLowerCase()) { return; } + this._loading = true; + this._changeNum = value.changeNum; this._patchRange = { patchNum: value.patchNum, @@ -225,7 +258,17 @@ return; } - this.$.diff.reload(); + var promises = []; + + promises.push(this._getDiffPreferences().then(function(prefs) { + this._prefs = prefs; + }.bind(this))); + + promises.push(this.$.diff.reload()); + + Promise.all(promises).then(function() { + this._loading = false; + }.bind(this)); }, _pathChanged: function(path) { @@ -283,6 +326,10 @@ return path == currentPath; }, + _computePrefsButtonHidden: function(prefs, loggedIn) { + return !loggedIn || !prefs; + }, + _computeKeyNav: function(path, selectedPath, fileList) { var selectedIndex = fileList.indexOf(selectedPath); if (fileList.indexOf(path) == selectedIndex - 1) { @@ -307,5 +354,36 @@ _showDropdownTapHandler: function(e) { this.$.dropdown.open(); }, + + _handlePrefsTap: function(e) { + e.preventDefault(); + this.$.prefsOverlay.open(); + }, + + _handlePrefsSave: function(e) { + e.stopPropagation(); + var el = Polymer.dom(e).rootTarget; + el.disabled = true; + this._saveDiffPreferences().then(function(response) { + el.disabled = false; + if (!response.ok) { + alert('Oops. Something went wrong. Check the console and bug the ' + + 'PolyGerrit team for assistance.'); + return response.text().then(function(text) { + console.error(text); + }); + } + this.$.prefsOverlay.close(); + }.bind(this)); + }, + + _saveDiffPreferences: function() { + return this.$.restAPI.saveDiffPreferences(this._prefs); + }, + + _handlePrefsCancel: function(e) { + e.stopPropagation(); + this.$.prefsOverlay.close(); + }, }); })();
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 1e7b0e4..2c70ef1 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -16,13 +16,8 @@ <link rel="import" href="../../../bower_components/polymer/polymer.html"> <link rel="import" href="../../shared/gr-button/gr-button.html"> -<link rel="import" href="../../shared/gr-overlay/gr-overlay.html"> -<link rel="import" href="../../shared/gr-request/gr-request.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> - <link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html"> -<link rel="import" href="../gr-diff-preferences/gr-diff-preferences.html"> -<link rel="import" href="../gr-patch-range-select/gr-patch-range-select.html"> <dom-module id="gr-diff"> <template> @@ -33,18 +28,6 @@ --light-add-highlight-color: #efe; --dark-add-highlight-color: #d4ffd4; } - .loading { - padding: 0 var(--default-horizontal-margin) 1em; - color: #666; - } - .header { - display: flex; - justify-content: space-between; - margin: 0 var(--default-horizontal-margin) .75em; - } - .prefsButton { - text-align: right; - } .diffContainer { border-bottom: 1px solid #eee; border-top: 1px solid #eee; @@ -148,33 +131,11 @@ content: '\00BB'; } </style> - <div class="loading" hidden$="[[!_loading]]">Loading...</div> - <div hidden$="[[_loading]]" hidden> - <div class="header"> - <gr-patch-range-select - path="[[path]]" - change-num="[[changeNum]]" - patch-range="[[patchRange]]" - available-patches="[[availablePatches]]"></gr-patch-range-select> - <gr-button link - class="prefsButton" - on-tap="_handlePrefsTap" - hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]" - hidden>Diff View Preferences</gr-button> - </div> - <gr-overlay id="prefsOverlay" with-backdrop> - <gr-diff-preferences - prefs="{{_prefs}}" - on-save="_handlePrefsSave" - on-cancel="_handlePrefsCancel"></gr-diff-preferences> - </gr-overlay> - - <div class$="[[_computeContainerClass(_loggedIn, _viewMode)]]" - on-tap="_handleTap" - on-mousedown="_handleMouseDown" - on-copy="_handleCopy"> - <table id="diffTable"></table> - </div> + <div class$="[[_computeContainerClass(_loggedIn, _viewMode)]]" + on-tap="_handleTap" + on-mousedown="_handleMouseDown" + on-copy="_handleCopy"> + <table id="diffTable"></table> </div> <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> </template>
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 09b83b2..b93b7a3 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -34,11 +34,10 @@ */ properties: { - availablePatches: Array, changeNum: String, patchRange: Object, path: String, - + prefs: Object, projectConfig: { type: Object, observer: '_projectConfigChanged', @@ -48,17 +47,12 @@ type: Boolean, value: false, }, - _loading: { - type: Boolean, - value: true, - }, _viewMode: { type: String, value: DiffViewMode.SIDE_BY_SIDE, }, _diff: Object, _diffBuilder: Object, - _prefs: Object, _selectionSide: { type: String, observer: '_selectionSideChanged', @@ -75,7 +69,7 @@ }, observers: [ - '_prefsChanged(_prefs.*)', + '_prefsChanged(prefs.*)', ], attached: function() { @@ -91,25 +85,21 @@ reload: function() { this._clearDiffContent(); - this._loading = true; var promises = []; promises.push(this._getDiff().then(function(diff) { this._diff = diff; - this._loading = false; }.bind(this))); promises.push(this._getDiffCommentsAndDrafts().then(function(comments) { this._comments = comments; }.bind(this))); - promises.push(this._getDiffPreferences().then(function(prefs) { - this._prefs = prefs; - }.bind(this))); - return Promise.all(promises).then(function() { - this._render(); + if (this.prefs) { + this._render(); + } }.bind(this)); }, @@ -202,41 +192,6 @@ return classes.join(' '); }, - _computePrefsButtonHidden: function(prefs, loggedIn) { - return !loggedIn || !prefs; - }, - - _handlePrefsTap: function(e) { - e.preventDefault(); - this.$.prefsOverlay.open(); - }, - - _handlePrefsSave: function(e) { - e.stopPropagation(); - var el = Polymer.dom(e).rootTarget; - el.disabled = true; - this._saveDiffPreferences().then(function(response) { - el.disabled = false; - if (!response.ok) { - alert('Oops. Something went wrong. Check the console and bug the ' + - 'PolyGerrit team for assistance.'); - return response.text().then(function(text) { - console.error(text); - }); - } - this.$.prefsOverlay.close(); - }.bind(this)); - }, - - _saveDiffPreferences: function() { - return this.$.restAPI.saveDiffPreferences(this._prefs); - }, - - _handlePrefsCancel: function(e) { - e.stopPropagation(); - this.$.prefsOverlay.close(); - }, - _handleTap: function(e) { var el = Polymer.dom(e).rootTarget; @@ -402,7 +357,7 @@ _render: function() { this._clearDiffContent(); this._builder = this._getDiffBuilder(this._diff, this._comments, - this._prefs); + this.prefs); this._builder.emitDiff(this._diff.content); this.async(function() { @@ -460,32 +415,6 @@ }).then(this._normalizeDiffCommentsAndDrafts.bind(this)); }, - _getDiffPreferences: function() { - return this._getLoggedIn().then(function(loggedIn) { - if (!loggedIn) { - // These defaults should match the defaults in - // gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java - // NOTE: There are some settings that don't apply to PolyGerrit - // (Render mode being at least one of them). - return Promise.resolve({ - auto_hide_diff_table_header: true, - context: 10, - cursor_blink_rate: 0, - ignore_whitespace: 'IGNORE_NONE', - intraline_difference: true, - line_length: 100, - show_line_endings: true, - show_tabs: true, - show_whitespace_errors: true, - syntax_highlighting: true, - tab_size: 8, - theme: 'DEFAULT', - }); - } - return this.$.restAPI.getDiffPreferences(); - }.bind(this)); - }, - _normalizeDiffCommentsAndDrafts: function(results) { function markAsDraft(d) { d.__draft = true;