Merge "Fix diff mode selector initial state"

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

@@ -334,6 +334,12 @@
 
     _resetFileListViewState: function() {
       this.set('viewState.selectedFileIndex', 0);
+      if (!!this.viewState.changeNum &&
+          this.viewState.changeNum !== this._changeNum) {
+        // Reset the diff mode to null when navigating from one change to
+        // another, so that the user's preference is restored.
+        this.set('viewState.diffMode', null);
+      }
       this.set('viewState.changeNum', this._changeNum);
       this.set('viewState.patchRange', this._patchRange);
     },

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

@@ -165,6 +165,36 @@ limitations under the License.
       assert.deepEqual(element._diffDrafts, {});
     });
 
+    test('change num change', function() {
+      element._changeNum = null;
+      element._patchRange = {
+        basePatchNum: 'PARENT',
+        patchNum: 2,
+      };
+      element._change = {
+        change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+        labels: {},
+      };
+      element.viewState.changeNum = null;
+      element.viewState.diffMode = 'UNIFIED';
+      flushAsynchronousOperations();
+      assert.equal(element.viewState.diffMode, 'UNIFIED');
+
+      element._changeNum = '1';
+      element.params = {changeNum: '1'};
+      element._change.newProp = '1';
+      flushAsynchronousOperations();
+      assert.equal(element.viewState.diffMode, 'UNIFIED');
+      assert.equal(element.viewState.changeNum, '1');
+
+      element._changeNum = '2';
+      element.params = {changeNum: '2'};
+      element._change.newProp = '2';
+      flushAsynchronousOperations();
+      assert.isNull(element.viewState.diffMode);
+      assert.equal(element.viewState.changeNum, '2');
+    });
+
     test('patch num change', function(done) {
       element._changeNum = '42';
       element._patchRange = {
@@ -183,7 +213,9 @@ limitations under the License.
         status: 'NEW',
         labels: {},
       };
+      element.viewState.diffMode = 'UNIFIED';
       flushAsynchronousOperations();
+
       var selectEl = element.$$('.patchInfo-header select');
       assert.ok(selectEl);
       var optionEls = Polymer.dom(element.root).querySelectorAll(
@@ -201,6 +233,7 @@ limitations under the License.
 
       var numEvents = 0;
       selectEl.addEventListener('change', function(e) {
+        assert.equal(element.viewState.diffMode, 'UNIFIED');
         numEvents++;
         if (numEvents == 1) {
           assert(showStub.lastCall.calledWithExactly('/c/42/1'),

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

@@ -105,6 +105,15 @@
         }
       }.bind(this));
 
+      if (this.changeViewState.diffMode === null) {
+        // Initialize with user's diff mode preference. Default to
+        // SIDE_BY_SIDE in the meantime.
+        this.set('changeViewState.diffMode', DiffViewMode.SIDE_BY_SIDE);
+        this.$.restAPI.getPreferences().then(function(prefs) {
+          this.set('changeViewState.diffMode', prefs.diff_view);
+        }.bind(this));
+      }
+
       if (this._path) {
         this.fire('title-change',
             {title: this._computeFileDisplayName(this._path)});
@@ -113,11 +122,6 @@
       this.$.cursor.push('diffs', this.$.diff);
     },
 
-    detached: function() {
-      // Reset the diff mode to null so that it reverts to the user preference.
-      this.changeViewState.diffMode = null;
-    },
-
     _getLoggedIn: function() {
       return this.$.restAPI.getLoggedIn();
     },

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

@@ -32,6 +32,12 @@ limitations under the License.
   </template>
 </test-fixture>
 
+<test-fixture id="blank">
+  <template>
+    <div></div>
+  </template>
+</text-fixture>
+
 <script>
   suite('gr-diff-view tests', function() {
     var element;
@@ -394,6 +400,29 @@ limitations under the License.
       assert.equal(element._getDiffViewMode(), diffDisplay.viewMode);
     });
 
+    test('diff mode selector initializes from preferences', function() {
+      var resolvePrefs;
+      var prefsPromise = new Promise(function(resolve) {
+        resolvePrefs = resolve;
+      });
+      var getPreferencesStub = sinon.stub(element.$.restAPI, 'getPreferences',
+          function() { return prefsPromise; });
+
+      // Attach a new gr-diff-view so we can intercept the preferences fetch.
+      var view = document.createElement('gr-diff-view');
+      var select = view.$.modeSelect;
+      fixture('blank').appendChild(view);
+      flushAsynchronousOperations();
+
+      // At this point the diff mode doesn't yet have the user's preference.
+      assert.equal(select.value, 'SIDE_BY_SIDE');
+
+      // Receive the overriding preference.
+      resolvePrefs({diff_view: 'UNIFIED'});
+      flushAsynchronousOperations();
+      assert.equal(select.value, 'SIDE_BY_SIDE');
+    });
+
     test('_loadHash', function() {
       assert.isNotOk(element.$.cursor.initialLineNumber);