|
|
Created:
6 years, 3 months ago by mmccoy Modified:
6 years, 3 months ago Reviewers:
Ted C, Pam (message me for reviews), arv (Not doing code reviews), Dan Beam, kerz_chromium CC:
chromium-reviews, arv+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Project:
chromium Visibility:
Public. |
DescriptionUpdate chrome://welcome resources with Material/M37 look and feel
BUG=399456
Committed: https://crrev.com/5ce3577e3fb5e043dc13229c06423823c60f7abc
Cr-Commit-Position: refs/heads/master@{#296186}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Linting and addressing CL comments #Patch Set 3 : Don't use remote resource url for font-face #
Total comments: 2
Patch Set 4 : CSS lint fixes #
Messages
Total messages: 24 (8 generated)
mmccoy@chromium.org changed reviewers: + tedchoc@chromium.org
Hi Ted, In relation to crbug/399456, updates the clank welcome splash page.
lgtm
The CQ bit was checked by mmccoy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/575213002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
mmccoy@chromium.org changed reviewers: + arv@chromium.org, dbeam@chromium.org
mmccoy@chromium.org changed reviewers: + pam@chromium.org
The CQ bit was checked by kerz@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/575213002/1
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/575213002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... File chrome/browser/resources/about_welcome_android/about_welcome_android.css (right): https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... chrome/browser/resources/about_welcome_android/about_welcome_android.css:5: @import url('//fonts.googleapis.com/css?family=RobotoDraft:regular,bold,italic,thin,light,bolditalic,black,medium'); why is this a remote URL? https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... chrome/browser/resources/about_welcome_android/about_welcome_android.css:46: background-color: rgb(66, 133, 244); alphabetize
https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... File chrome/browser/resources/about_welcome_android/about_welcome_android.css (right): https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... chrome/browser/resources/about_welcome_android/about_welcome_android.css:5: @import url('//fonts.googleapis.com/css?family=RobotoDraft:regular,bold,italic,thin,light,bolditalic,black,medium'); On 2014/09/19 21:43:39, Dan Beam wrote: > why is this a remote URL? Yeah, I wasn't sure about this -- the design calls for Roboto2 as the font face, and my understanding is that the system font will still be Droid Sans until L ships, so I chose to use a remote resource until then. Let me know if I should just fall back to system font anyway. https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... chrome/browser/resources/about_welcome_android/about_welcome_android.css:46: background-color: rgb(66, 133, 244); On 2014/09/19 21:43:39, Dan Beam wrote: > alphabetize Done.
On 2014/09/21 13:32:34, mmccoy wrote: > https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... > File chrome/browser/resources/about_welcome_android/about_welcome_android.css > (right): > > https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... > chrome/browser/resources/about_welcome_android/about_welcome_android.css:5: > @import > url('//fonts.googleapis.com/css?family=RobotoDraft:regular,bold,italic,thin,light,bolditalic,black,medium'); > On 2014/09/19 21:43:39, Dan Beam wrote: > > why is this a remote URL? > > Yeah, I wasn't sure about this -- the design calls for Roboto2 as the font face, > and my understanding is that the system font will still be Droid Sans until L > ships, so I chose to use a remote resource until then. Let me know if I should > just fall back to system font anyway. After further review and looking at this cl (https://codereview.chromium.org/519453002/), this seems like the right way to do it. > https://codereview.chromium.org/575213002/diff/1/chrome/browser/resources/abo... > chrome/browser/resources/about_welcome_android/about_welcome_android.css:46: > background-color: rgb(66, 133, 244); > On 2014/09/19 21:43:39, Dan Beam wrote: > > alphabetize > > Done.
lgtm https://codereview.chromium.org/575213002/diff/40001/chrome/browser/resources... File chrome/browser/resources/about_welcome_android/about_welcome_android.css (right): https://codereview.chromium.org/575213002/diff/40001/chrome/browser/resources... chrome/browser/resources/about_welcome_android/about_welcome_android.css:51: color: #fff; nit: white
Thanks for the review, Dan. https://codereview.chromium.org/575213002/diff/40001/chrome/browser/resources... File chrome/browser/resources/about_welcome_android/about_welcome_android.css (right): https://codereview.chromium.org/575213002/diff/40001/chrome/browser/resources... chrome/browser/resources/about_welcome_android/about_welcome_android.css:51: color: #fff; On 2014/09/23 05:57:49, Dan Beam wrote: > nit: white Done.
The CQ bit was checked by mmccoy@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/575213002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as b01559b0baa1732564a03412de5663d57ef945a4
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/5ce3577e3fb5e043dc13229c06423823c60f7abc Cr-Commit-Position: refs/heads/master@{#296186} |