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
2016-04-23 23:18:43 -04:00
parent 387e3a4257
commit c80291ca05
4 changed files with 135 additions and 130 deletions
--- 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 @@ limitations under the License.
 <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 @@ limitations under the License.
        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 @@ limitations under the License.
        </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>
--- 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();
+    },
  });
 })();
--- 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 @@ limitations under the License.

 <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 @@ limitations under the License.
        --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 @@ limitations under the License.
        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>
--- 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;