Commit 5730340b authored by Malay Keshav's avatar Malay Keshav Committed by Commit Bot

Fix immediate pref value change in display resolution settings

We use a change observer to trigger updating the selected mode of a
display. This is useful for the drop down menu we use to select the mode
on external displays. However, if display zoom is disabled, we show a
slider instead of a drop down. The slider triggers the same pref value
change even if the user is still dragging it. This causes a usability
regression on the settings resolution slider.

To fix this, the patch adds a check which ensures we only listen to
events fired due to releasing the slider when display zoom is disabled.

Bug: 845712
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9965bdf3121289ac29f38175cc9ad179cda41497
Component: Display settings, resolution slider
Reviewed-on: https://chromium-review.googlesource.com/1069773
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562555}
parent 732761ac
......@@ -148,7 +148,7 @@
</div>
<settings-slider disabled="[[!enableSetResolution_(selectedDisplay)]]"
tick-values="[[modeValues_]]" pref="{{selectedModePref_}}"
on-change="onSelectedModeChange_">
on-change="onSelectedModeSliderChange_">
</settings-slider>
</div>
</template>
......
......@@ -692,16 +692,11 @@ Polymer({
},
/**
* Triggered when the 'change' event for the selected mode slider is
* triggered. This only occurs when the value is committed (i.e. not while
* the slider is being dragged).
* @param {number} newModeIndex The new index value for which thie function is
* called.
* Updates the selected mode based on the latest pref value.
* @private
*/
onSelectedModeChange_: function(newModeIndex) {
onSelectedModeSliderChange_: function() {
if (this.currentSelectedModeIndex_ == -1 ||
this.currentSelectedModeIndex_ == newModeIndex ||
this.currentSelectedModeIndex_ == this.selectedModePref_.value) {
// Don't change the selected display mode until we have received an update
// from Chrome and the mode differs from the current mode.
......@@ -716,6 +711,24 @@ Polymer({
this.setPropertiesCallback_.bind(this));
},
/**
* Handles a change in the |selectedModePref| value triggered via the observer
* @param {number} newModeIndex The new index value for which thie function is
* called.
* @private
*/
onSelectedModeChange_: function(newModeIndex) {
// We want to ignore all value changes to the pref due to the slider being
// dragged. Since this can only happen when the slider is present which is
// when display zoom is disabled, we can use this check.
// See http://crbug/845712 for more info.
if (!this.showDisplayZoomSetting_)
return;
if (this.currentSelectedModeIndex_ == newModeIndex)
return;
this.onSelectedModeSliderChange_();
},
/**
* Triggerend when the display size slider changes its value. This only
* occurs when the value is committed (i.e. not while the slider is being
......
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