Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2032683003:
Add ripples to md checkboxes/radio buttons (Closed)
|
Created:
4 years, 6 months ago by Evan Stade Modified:
4 years, 5 months ago CC:
chromium-reviews, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd ripples to md checkboxes/radio buttons
Not entirely sure this is what Sebastien had in mind, but it seems
like a reasonable starting point. Will ping him to test it out after
landing.
BUG=609922
Committed: https://crrev.com/4e38e0aa9cfd669abc777748cc47646d3cb8ea46
Cr-Commit-Position: refs/heads/master@{#404426}
Patch Set 1 #
Total comments: 3
Patch Set 2 : rebase #
Messages
Total messages: 21 (8 generated)
estade@chromium.org changed reviewers: + bruthig@chromium.org, sky@chromium.org
LGTM https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... File ui/views/controls/button/checkbox.cc (right): https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... ui/views/controls/button/checkbox.cc:178: ui::NativeTheme::kColorId_UnfocusedBorderColor); This seems like a weird choice for a base color. I'm assuming you know what you're doing.
lgtm
https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... File ui/views/controls/button/checkbox.cc (right): https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... ui/views/controls/button/checkbox.cc:178: ui::NativeTheme::kColorId_UnfocusedBorderColor); On 2016/06/02 03:13:30, sky wrote: > This seems like a weird choice for a base color. How so? It's the color of the checkbox square/radio circle. In other places like the bookmark bar folder buttons we use the color of the icon, so this seems like the most natural choice. > I'm assuming you know what > you're doing. I hope you're right.
https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... File ui/views/controls/button/checkbox.cc (right): https://codereview.chromium.org/2032683003/diff/1/ui/views/controls/button/ch... ui/views/controls/button/checkbox.cc:178: ui::NativeTheme::kColorId_UnfocusedBorderColor); On 2016/06/02 19:40:18, Evan Stade wrote: > On 2016/06/02 03:13:30, sky wrote: > > This seems like a weird choice for a base color. > > How so? It's the color of the checkbox square/radio circle. In other places like > the bookmark bar folder buttons we use the color of the icon, so this seems like > the most natural choice. I didn't realize we use 'unfocused border color' in those places. IMO 'unfocused border color' isn't a descriptive name for a color used as a base in many places. But I realize naming is hard. > > > I'm assuming you know what > > you're doing. > > I hope you're right.
The CQ bit was checked by estade@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by estade@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org, bruthig@chromium.org Link to the patchset: https://codereview.chromium.org/2032683003/#ps20001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by estade@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Add ripples to md checkboxes/radio buttons Not entirely sure this is what Sebastien had in mind, but it seems like a reasonable starting point. Will ping him to test it out after landing. BUG=609922 ========== to ========== Add ripples to md checkboxes/radio buttons Not entirely sure this is what Sebastien had in mind, but it seems like a reasonable starting point. Will ping him to test it out after landing. BUG=609922 Committed: https://crrev.com/4e38e0aa9cfd669abc777748cc47646d3cb8ea46 Cr-Commit-Position: refs/heads/master@{#404426} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/4e38e0aa9cfd669abc777748cc47646d3cb8ea46 Cr-Commit-Position: refs/heads/master@{#404426} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
