Commit 21b13d12 authored by Ahmed Fakhry's avatar Ahmed Fakhry Committed by Commit Bot

MD Settings: Fix a resolution slider bug

Moving from a tab of a display that has more modes to a tab
of another display that has less modes, where the selected
mode of the first display has index >= max index of the second
display's modes, this would result in the first display's mode
changing to the previous one in the slider.

This CL fixes this issue by ignoring change callbacks when the
various parameters of the currently selected display are still
out of sync.

BUG=822248

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I97d1a55b87ad99d9d6821270618609a7c4a880a7
Reviewed-on: https://chromium-review.googlesource.com/965630
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543903}
parent da666665
...@@ -271,7 +271,8 @@ Polymer({ ...@@ -271,7 +271,8 @@ Polymer({
/** /**
* @param {!chrome.system.display.DisplayUnitInfo} selectedDisplay * @param {!chrome.system.display.DisplayUnitInfo} selectedDisplay
* @return {number} * @return {number} The index of the currently selected mode of the
* |selectedDisplay|. If the display has no modes, returns 0.
* @private * @private
*/ */
getSelectedModeIndex_: function(selectedDisplay) { getSelectedModeIndex_: function(selectedDisplay) {
...@@ -351,19 +352,20 @@ Polymer({ ...@@ -351,19 +352,20 @@ Polymer({
* @private * @private
*/ */
setSelectedDisplay_: function(selectedDisplay) { setSelectedDisplay_: function(selectedDisplay) {
// Set |currentSelectedModeIndex_| and |modeValues_| first since these // |modeValues_| controls the resolution slider's tick values. Changing it
// are not used directly in data binding. // might trigger a change in the |selectedModePref_.value| if the number of
// modes differs and the current mode index is out of range of the new modes
// indices. Thus, we need to set |currentSelectedModeIndex_| to -1 to
// indicate that the |selectedDisplay| and |selectedModePref_.value| are out
// of sync, and therefore getResolutionText_() and onSelectedModeChange_()
// will be no-ops.
this.currentSelectedModeIndex_ = -1;
const numModes = selectedDisplay.modes.length; const numModes = selectedDisplay.modes.length;
if (numModes == 0) { this.modeValues_ = numModes == 0 ? [] : Array.from(Array(numModes).keys());
this.modeValues_ = [];
this.currentSelectedModeIndex_ = 0;
} else {
this.modeValues_ = Array.from(Array(numModes).keys());
this.currentSelectedModeIndex_ =
this.getSelectedModeIndex_(selectedDisplay);
}
if (this.showDisplayZoomSetting_) { if (this.showDisplayZoomSetting_) {
// Note that the display zoom values has the same number of ticks for all
// displays, so the above problem doesn't apply here.
this.zoomValues_ = this.getZoomValues_(selectedDisplay); this.zoomValues_ = this.getZoomValues_(selectedDisplay);
this.set( this.set(
'selectedZoomPref_.value', 'selectedZoomPref_.value',
...@@ -374,6 +376,11 @@ Polymer({ ...@@ -374,6 +376,11 @@ Polymer({
// Set |selectedDisplay| first since only the resolution slider depends // Set |selectedDisplay| first since only the resolution slider depends
// on |selectedModePref_|. // on |selectedModePref_|.
this.selectedDisplay = selectedDisplay; this.selectedDisplay = selectedDisplay;
// Now that everything is in sync, set the selected mode to its correct
// value right before updating the pref.
this.currentSelectedModeIndex_ =
this.getSelectedModeIndex_(selectedDisplay);
this.set('selectedModePref_.value', this.currentSelectedModeIndex_); this.set('selectedModePref_.value', this.currentSelectedModeIndex_);
}, },
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment