Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2477693004:
Change color of ripple effects and tab focus of toolbar buttons in Files (Closed)
|
Created:
4 years, 1 month ago by yamaguchi Modified:
4 years, 1 month ago CC:
chromium-reviews, oka+watch_chromium.org, rginda+watch_chromium.org, mtomasz+watch_chromium.org, yamaguchi+watch_chromium.org, fukino+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChange color of ripple effects and tab focus of toolbar buttons in Files
and Gallery apps to have high contrast to the backgrounds.
BUG=660353
TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/02a288c7de72990084861509042542fa3acfb71b
Cr-Commit-Position: refs/heads/master@{#430528}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix highlight colors. #Patch Set 3 : Fix highlight color. #Patch Set 4 : Change color in files-ripple for buttons that don't have lock status. #
Total comments: 2
Patch Set 5 : Explicitly set the ripple color on media control as media control is drawn on white background. #Patch Set 6 : Make ripple color of non-toggle buttons customizable. Fix ripple colors in Audio Player. #
Total comments: 1
Patch Set 7 : Fix ripple color of video player. #
Total comments: 2
Patch Set 8 : Sort by the lexicological order of the selectors. #
Messages
Total messages: 44 (29 generated)
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to keep high contrast from the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to keep high contrast from the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted properly. Also hit the sort and search buttons to see these are highlighted while accepting input there. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
yamaguchi@chromium.org changed reviewers: + yawano@chromium.org
ptal
https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/css/file_manager.css (right): https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/css/file_manager.css:451: background-color: rgba(255, 0, 0, 0.20); rgba(255, 255, 255, 0.20)? https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/elements/files_toggle_ripple.html (right): https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/elements/files_toggle_ripple.html:29: background-color: #FFF; nit: use named color, i.e. white. - https://www.chromium.org/developers/web-development-style-guide
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/css/file_manager.css (right): https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/css/file_manager.css:451: background-color: rgba(255, 0, 0, 0.20); On 2016/11/04 06:08:28, yawano wrote: > rgba(255, 255, 255, 0.20)? Done. https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/elements/files_toggle_ripple.html (right): https://codereview.chromium.org/2477693004/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/elements/files_toggle_ripple.html:29: background-color: #FFF; On 2016/11/04 06:08:28, yawano wrote: > nit: use named color, i.e. white. - > https://www.chromium.org/developers/web-development-style-guide Done.
I found that files_ripple.html also needs change because the buttons that are not "toggle" (e.g. delete button) are made of files_ripple elements, and added to this changelist. ptal.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/elements/files_ripple.html (right): https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/elements/files_ripple.html:22: background-color: white; files-ripple is used also for video player. Is it intended? - https://cs.chromium.org/chromium/src/ui/file_manager/video_player/js/media_co...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/elements/files_ripple.html (right): https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/elements/files_ripple.html:22: background-color: white; On 2016/11/04 09:14:29, yawano wrote: > files-ripple is used also for video player. Is it intended? - > https://cs.chromium.org/chromium/src/ui/file_manager/video_player/js/media_co... I confirmed that it is used by video player via "files-icon-button" dom-module. This will affect the playback control buttons there. I added a fix to avoid that effect. (I also tried to fix it by introducing custom properties, but not got good result yet. I may ask about this offline later.)
On 2016/11/04 15:27:03, yamaguchi wrote: > https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... > File ui/file_manager/file_manager/foreground/elements/files_ripple.html (right): > > https://codereview.chromium.org/2477693004/diff/60001/ui/file_manager/file_ma... > ui/file_manager/file_manager/foreground/elements/files_ripple.html:22: > background-color: white; > On 2016/11/04 09:14:29, yawano wrote: > > files-ripple is used also for video player. Is it intended? - > > > https://cs.chromium.org/chromium/src/ui/file_manager/video_player/js/media_co... > > I confirmed that it is used by video player via "files-icon-button" dom-module. > This will affect the playback control buttons there. I added a fix to avoid that > effect. (I also tried to fix it by introducing custom properties, but not got > good result yet. I may ask about this offline later.) I guess audio player needs to be updated too. https://codesearch.chromium.org/search/?q=files-icon-button&sq=package:chromi...
https://codereview.chromium.org/2477693004/diff/100001/ui/file_manager/video_... File ui/file_manager/video_player/css/media_controls.css (right): https://codereview.chromium.org/2477693004/diff/100001/ui/file_manager/video_... ui/file_manager/video_player/css/media_controls.css:132: .video-controls .ripple { Now I am seeing this doesn't work because style for .files-toggle-ripple-0 .ripple.files-toggle-ripple takes higher precedence. It is affected by the addition of @apply(--files-ripple); to the files_ripple.html by patchset 6.
On 2016/11/07 09:15:13, yamaguchi wrote: > https://codereview.chromium.org/2477693004/diff/100001/ui/file_manager/video_... > File ui/file_manager/video_player/css/media_controls.css (right): > > https://codereview.chromium.org/2477693004/diff/100001/ui/file_manager/video_... > ui/file_manager/video_player/css/media_controls.css:132: .video-controls .ripple > { > Now I am seeing this doesn't work because style for .files-toggle-ripple-0 > .ripple.files-toggle-ripple takes higher precedence. > It is affected by the addition of @apply(--files-ripple); to the > files_ripple.html by patchset 6. Done.
yamaguchi@chromium.org changed reviewers: + fukino@chromium.org
ptal
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: 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
Dry run: This issue passed the CQ dry run.
oka@chromium.org changed reviewers: + oka@chromium.org
lgtm https://codereview.chromium.org/2477693004/diff/120001/ui/file_manager/video_... File ui/file_manager/video_player/video_player.html (right): https://codereview.chromium.org/2477693004/diff/120001/ui/file_manager/video_... ui/file_manager/video_player/video_player.html:26: files-icon-button { nit: move this above paper-slider to sort lexicographically.
https://codereview.chromium.org/2477693004/diff/120001/ui/file_manager/video_... File ui/file_manager/video_player/video_player.html (right): https://codereview.chromium.org/2477693004/diff/120001/ui/file_manager/video_... ui/file_manager/video_player/video_player.html:26: files-icon-button { On 2016/11/07 10:24:32, oka wrote: > nit: move this above paper-slider to sort lexicographically. Done.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: 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
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yamaguchi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from oka@chromium.org Link to the patchset: https://codereview.chromium.org/2477693004/#ps140001 (title: "Sort by the lexicological order of the selectors.")
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.
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Change color of ripple effects and tab focus of toolbar buttons in Files and Gallery apps to have high contrast to the backgrounds. BUG=660353 TEST=Open files app and hit tab key several times to see tab focus is highlighted white. Also hit the sort and search buttons to see these are highlighted in the same way while the dropdown or text input is displayed there respectively. Open an image file in Gallery app to do the same with "edit" button. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/02a288c7de72990084861509042542fa3acfb71b Cr-Commit-Position: refs/heads/master@{#430528} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/02a288c7de72990084861509042542fa3acfb71b Cr-Commit-Position: refs/heads/master@{#430528} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
