Merge "Add toggle to PolyGerrit diff view to switch between diff styles"

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html

@@ -103,6 +103,9 @@ limitations under the License.
       .prefsButton {
         text-align: right;
       }
+      #modeSelect {
+        margin-left: .5em;
+      }
       @media screen and (max-width: 50em) {
         .dash {
           display: none;
@@ -168,11 +171,17 @@ limitations under the License.
             patch-range="[[_patchRange]]"
             available-patches="[[_computeAvailablePatches(_change.revisions)]]">
         </gr-patch-range-select>
-        <gr-button link
+        <div>
-           class="prefsButton"
+          <gr-button link
-           on-tap="_handlePrefsTap"
+              class="prefsButton"
-           hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
+              on-tap="_handlePrefsTap"
-           hidden>Diff View Preferences</gr-button>
+              hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]"
+              hidden>Diff View Preferences</gr-button>
+          <select id="modeSelect" on-change="_handleModeChange">
+            <option value="SIDE_BY_SIDE">Side By Side</option>
+            <option value="UNIFIED_DIFF">Unified</option>
+          </select>
+        </div>
       </div>
       <gr-overlay id="prefsOverlay" with-backdrop>
         <gr-diff-preferences
@@ -186,6 +195,7 @@ limitations under the License.
           path="[[_path]]"
           prefs="[[_prefs]]"
           project-config="[[_projectConfig]]"
+          view-mode="[[_diffMode]]"
           on-render="_handleDiffRender">
       </gr-diff>
     </div>

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js

@@ -16,6 +16,11 @@
 
   var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
 
+  var DiffViewMode = {
+    SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+    UNIFIED: 'UNIFIED_DIFF',
+  };
+
   Polymer({
     is: 'gr-diff-view',
 
@@ -64,6 +69,11 @@
         value: true,
       },
       _prefs: Object,
+      _userPrefs: Object,
+      _diffMode: {
+        type: String,
+        computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)'
+      },
     },
 
     behaviors: [
@@ -74,6 +84,7 @@
       '_getChangeDetail(_changeNum)',
       '_getProjectConfig(_change.project)',
       '_getFiles(_changeNum, _patchRange.patchNum)',
+      '_updateModeSelect(_diffMode)',
     ],
 
     attached: function() {
@@ -92,6 +103,9 @@
     },
 
     detached: function() {
+      // Reset the diff mode to null so that it reverts to the user preference.
+      this.changeViewState.diffMode = null;
+
       window.removeEventListener('resize', this._boundWindowResizeHandler);
     },
 
@@ -124,6 +138,10 @@
       return this.$.restAPI.getDiffPreferences();
     },
 
+    _getPreferences: function() {
+      return this.$.restAPI.getPreferences();
+    },
+
     _handleReviewedChange: function(e) {
       this._setReviewed(Polymer.dom(e).rootTarget.checked);
     },
@@ -242,6 +260,10 @@
         this._prefs = prefs;
       }.bind(this)));
 
+      promises.push(this._getPreferences().then(function(prefs) {
+        this._userPrefs = prefs;
+      }.bind(this)));
+
       promises.push(this.$.diff.reload());
 
       Promise.all(promises).then(function() {
@@ -360,5 +382,43 @@
       e.stopPropagation();
       this.$.prefsOverlay.close();
     },
+
+    _handleModeChange: function(e) {
+      this.set('changeViewState.diffMode', this.$.modeSelect.value);
+    },
+
+    /**
+     * _getDiffViewMode: Get the diff view (side-by-side or unified) based on
+     * the current state.
+     *
+     * The expected behavior is to use the mode specified in the user's
+     * preferences unless they have manually chosen the alternative view. If the
+     * user navigates up to the change view, it should clear this choice and
+     * revert to the preference the next time a diff is viewed.
+     *
+     * Use side-by-side if the user is not logged in.
+     *
+     * @return {String}
+     */
+    _getDiffViewMode: function() {
+      if (this.changeViewState.diffMode) {
+        return this.changeViewState.diffMode;
+      } else if (this._userPrefs && this._userPrefs.diff_view) {
+        return this.changeViewState.diffMode = this._userPrefs.diff_view;
+      }
+
+      return DiffViewMode.SIDE_BY_SIDE;
+    },
+
+    /**
+     * Synchronize the mode select if it deviates from the selected mode state.
+     * This is mainly to keep it accurate when the page loads.
+     */
+    _updateModeSelect: function() {
+      var mode = this._getDiffViewMode();
+      if (this.$.modeSelect.value !== mode) {
+        this.$.modeSelect.value = mode;
+      }
+    },
   });
 })();

polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html

@@ -356,5 +356,37 @@ limitations under the License.
         done();
       });
     });
+
+    test('diff mode selector correctly toggles the diff', function() {
+      var select = element.$.modeSelect;
+      var diffDisplay = element.$.diff;
+
+      element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
+
+      // The mode selected in the view state reflects the selected option.
+      assert.equal(element._getDiffViewMode(), select.value);
+
+      // The mode selected in the view state reflects the view rednered in the
+      // diff.
+      assert.equal(select.value, diffDisplay.viewMode);
+
+      // We will simulate a user change of the selected mode.
+      var newMode = 'UNIFIED_DIFF';
+
+      // Listen to the change handler.
+      var eventStub = sinon.spy(element, '_handleModeChange');
+
+      // Set the actual value of the select, and simulate the change event.
+      select.value = newMode;
+      element.fire('change', {}, { node: select });
+
+      // Make sure the handler was called and the state is still coherent.
+      assert.isTrue(eventStub.called);
+      assert.equal(element._getDiffViewMode(), newMode);
+      assert.equal(element._getDiffViewMode(), select.value);
+      assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
+
+      eventStub.restore();
+    });
   });
 </script>

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html

@@ -131,7 +131,7 @@ limitations under the License.
         content: '\00BB';
       }
     </style>
-    <div class$="[[_computeContainerClass(_loggedIn, _viewMode)]]"
+    <div class$="[[_computeContainerClass(_loggedIn, viewMode)]]"
         on-tap="_handleTap"
         on-mousedown="_handleMouseDown"
         on-copy="_handleCopy">

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js

@@ -47,7 +47,7 @@
         type: Boolean,
         value: false,
       },
-      _viewMode: {
+      viewMode: {
         type: String,
         value: DiffViewMode.SIDE_BY_SIDE,
       },
@@ -69,7 +69,7 @@
     },
 
     observers: [
-      '_prefsChanged(prefs.*)',
+      '_prefsChanged(prefs.*, viewMode)',
     ],
 
     attached: function() {
@@ -439,14 +439,14 @@
     },
 
     _getDiffBuilder: function(diff, comments, prefs) {
-      if (this._viewMode === DiffViewMode.SIDE_BY_SIDE) {
+      if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
         return new GrDiffBuilderSideBySide(diff, comments, prefs,
             this.$.diffTable);
-      } else if (this._viewMode === DiffViewMode.UNIFIED) {
+      } else if (this.viewMode === DiffViewMode.UNIFIED) {
         return new GrDiffBuilderUnified(diff, comments, prefs,
             this.$.diffTable);
       }
-      throw Error('Unsupported diff view mode: ' + this._viewMode);
+      throw Error('Unsupported diff view mode: ' + this.viewMode);
     },
 
     _projectConfigChanged: function(projectConfig) {

polygerrit-ui/app/elements/gr-app.js

@@ -71,6 +71,7 @@
           patchNum: null,
           selectedFileIndex: 0,
           showReplyDialog: false,
+          diffMode: null,
         },
         changeListView: {
           query: null,

polygerrit-ui/app/test/index.html

@@ -49,6 +49,7 @@ limitations under the License.
     '../elements/diff/gr-diff-preferences/gr-diff-preferences_test.html',
     '../elements/diff/gr-diff-view/gr-diff-view_test.html',
     '../elements/diff/gr-patch-range-select/gr-patch-range-select_test.html',
+    '../elements/shared/gr-alert/gr-alert_test.html',
     '../elements/shared/gr-account-label/gr-account-label_test.html',
     '../elements/shared/gr-account-link/gr-account-link_test.html',
     '../elements/shared/gr-alert/gr-alert_test.html',