Replace display-size-slider with cr-slider or settings-slider |
||||
Issue descriptiondisplay-size-slider is used in CrOS for display size and some a11y settings. The main difference is the display size slider has a different label style. If we can add support for this label style in cr-slider or settings-slider, we could remove display-size-slider which would be better for maintenance reasons. The night-light-slider also uses a similar label style to the display-size-slider. The other difference is the night-light-slider has two knobs. This might be sufficiently different from cr-slider. We should still consider adding a common control like cr-range-slider or see if we can incorporate the functionality into cr-slider.
,
Oct 5
This change broke the display size slider. The setting updates the display size as soon as the slider is dragged, instead of waiting for a mouse/touch release. This makes it unusable.
,
Oct 5
Please ensure to re-add tests for this. Most of the tests for the slider seems to have been removed: https://chromium-review.googlesource.com/c/chromium/src/+/1211805/23/chrome/test/data/webui/settings/display_size_slider_test.js
,
Oct 6
,
Oct 8
Can we get this fixed as priority so it doesn't block the release schedule? Assigned properly with the proper cc's?
,
Oct 8
I have a CL with the fix here. https://chromium-review.googlesource.com/c/chromium/src/+/1266601
,
Oct 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db commit f2a5cc5e14eb4192acd90f5234e8ff95a8b733db Author: Esmael El-Moslimany <aee@chromium.org> Date: Mon Oct 08 23:46:48 2018 Settings WebUI: cr-slider,add support for not immediately updating value until drag stop Bug: 858882 Change-Id: I8845b5112c34dd9750e256e881b295bcce1b6047 Reviewed-on: https://chromium-review.googlesource.com/c/1266601 Commit-Queue: Esmael El-Moslimany <aee@chromium.org> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#597733} [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/chrome/browser/resources/settings/controls/settings_slider.html [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/chrome/browser/resources/settings/controls/settings_slider.js [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/chrome/browser/resources/settings/device_page/display.js [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/chrome/test/data/webui/cr_elements/cr_slider_test.js [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/chrome/test/data/webui/settings/device_page_tests.js [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/ui/webui/resources/cr_elements/cr_slider/cr_slider.html [modify] https://crrev.com/f2a5cc5e14eb4192acd90f5234e8ff95a8b733db/ui/webui/resources/cr_elements/cr_slider/cr_slider.js
,
Oct 9
FYI, this didn't land 'til 71.0.3575.0. Blocking DEV until this Chrome is picked up.
,
Oct 9
Can we tag as fixed pending verification to remove the block? Thanks
,
Oct 9
,
Oct 10
The original CL in #1 landed in 71.0.3565.0 with the fix in #7 landing in 71.0.3575.0. |
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Sep 28