Implementing Win 10 FRE visual design tweaks requested by first UX pass.

Implementing Win 10 FRE visual design tweaks requested by first UX pass.

BUG=662175
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2590253002
Cr-Commit-Position: refs/heads/master@{#442369}
Committed: https://chromium.googlesource.com/chromium/src/+/8890be37e55775f3fae49e71883c81a7c4ddd22a

[tmartino, 2016-12-20 16:00:30]

Description was changed from

==========
Implementing Win 10 FRE visual design tweaks requested by first UX pass.

BUG=662175
==========

to

==========
Implementing Win 10 FRE visual design tweaks requested by first UX pass.

BUG=662175
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tmartino, 2016-12-20 16:00:52]

The CQ bit was checked by tmartino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 16:01:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 17:49:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-20 17:49:08]

Dry run: This issue passed the CQ dry run.

[tmartino, 2016-12-20 17:52:23]

tmartino@chromium.org changed reviewers:
+ dbeam@chromium.org

[tmartino, 2016-12-20 17:52:24]

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:35: .header-logo {
Removing the image set because it doesn't seem to actually...work. In the CSS
that's eventually loaded by Chrome, the 2x version is omitted entirely:

content: -webkit-image-set(url(/logo-large.png) 1x);

For the logo it's probably OK to just always use the 2x version (these are
loaded locally anyway). Down the road it may cause problems with the remote
.webp files (see below), so if you have any idea why this is happening, let me
know.

[Dan Beam, 2016-12-21 03:07:55]

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (left):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:124: background:
var(--paper-blue-a200);
did this ever work?

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:35: .header-logo {
On 2016/12/20 17:52:24, tmartino wrote:
> Removing the image set because it doesn't seem to actually...work. In the CSS
> that's eventually loaded by Chrome, the 2x version is omitted entirely:
> 
> content: -webkit-image-set(url(/logo-large.png) 1x);
> 
> For the logo it's probably OK to just always use the 2x version (these are
> loaded locally anyway). Down the road it may cause problems with the remote
> .webp files (see below), so if you have any idea why this is happening, let me
> know.

did you try in 2x?  i.e. --force-device-scale-factor=2 or whatever?

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:134: text-transform:
uppercase;
why are you doing this?  doesn't paper-button have this by default?

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.html (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.html:80: <paper-button
class="button" on-tap="onContinue">$i18n{continueText}</paper-button>
80 col wrap

[tmartino, 2017-01-05 21:28:21]

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (left):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:124: background:
var(--paper-blue-a200);
On 2016/12/21 at 03:07:55, Dan Beam wrote:
> did this ever work?

I suspect not, but I'm not the original author.

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:35: .header-logo {
On 2016/12/21 at 03:07:55, Dan Beam wrote:
> On 2016/12/20 17:52:24, tmartino wrote:
> > Removing the image set because it doesn't seem to actually...work. In the
CSS
> > that's eventually loaded by Chrome, the 2x version is omitted entirely:
> > 
> > content: -webkit-image-set(url(/logo-large.png) 1x);
> > 
> > For the logo it's probably OK to just always use the 2x version (these are
> > loaded locally anyway). Down the road it may cause problems with the remote
> > .webp files (see below), so if you have any idea why this is happening, let
me
> > know.
> 
> did you try in 2x?  i.e. --force-device-scale-factor=2 or whatever?

Yeah, there's still some bizarre processing going on that ends up stripping out
the 2x version. In fact, if I reverse the order and put the 2x version first in
the CSS:

content: -webkit-image-set(url(chrome://welcome-win10/logo-large.png) 2x,
url(chrome://welcome-win10/logo-small.png) 1x);


Then what pops up in the rendered page is:

content: -webkit-image-set(, url(chrome://welcome-win10/logo-large.png) 1x);

Which breaks the image entirely.

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:134: text-transform:
uppercase;
On 2016/12/21 at 03:07:55, Dan Beam wrote:
> why are you doing this?  doesn't paper-button have this by default?

It appears it does. Removed.

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.html (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.html:80: <paper-button
class="button" on-tap="onContinue">$i18n{continueText}</paper-button>
On 2016/12/21 at 03:07:55, Dan Beam wrote:
> 80 col wrap

Done

[Dan Beam, 2017-01-06 00:55:41]

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
File chrome/browser/resources/welcome/win10/inline.css (right):

https://codereview.chromium.org/2590253002/diff/1/chrome/browser/resources/we...
chrome/browser/resources/welcome/win10/inline.css:35: .header-logo {
On 2017/01/05 21:28:21, tmartino wrote:
> On 2016/12/21 at 03:07:55, Dan Beam wrote:
> > On 2016/12/20 17:52:24, tmartino wrote:
> > > Removing the image set because it doesn't seem to actually...work. In the
> CSS
> > > that's eventually loaded by Chrome, the 2x version is omitted entirely:
> > > 
> > > content: -webkit-image-set(url(/logo-large.png) 1x);
> > > 
> > > For the logo it's probably OK to just always use the 2x version (these are
> > > loaded locally anyway). Down the road it may cause problems with the
remote
> > > .webp files (see below), so if you have any idea why this is happening,
let
> me
> > > know.
> > 
> > did you try in 2x?  i.e. --force-device-scale-factor=2 or whatever?
> 
> Yeah, there's still some bizarre processing going on that ends up stripping
out
> the 2x version.

that's probably grit

> In fact, if I reverse the order and put the 2x version first in
> the CSS:
> 
> content: -webkit-image-set(url(chrome://welcome-win10/logo-large.png) 2x,
> url(chrome://welcome-win10/logo-small.png) 1x);
> 
> 
> Then what pops up in the rendered page is:
> 
> content: -webkit-image-set(, url(chrome://welcome-win10/logo-large.png) 1x);
> 
> Which breaks the image entirely.

but did you try the proposed version (after this CL is applied) with
--force-device-scale-factor=2?

[tmartino, 2017-01-09 18:56:40]

On 2017/01/06 at 00:55:41, dbeam wrote:
> that's probably grit

I can file a bug against it and possibly look into it myself. Is there a grit
component or whatever?

> but did you try the proposed version (after this CL is applied) with
--force-device-scale-factor=2?

Yes, it looks correct.

[Dan Beam, 2017-01-09 19:44:52]

lgtm

[tmartino, 2017-01-09 19:46:16]

The CQ bit was checked by tmartino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 19:46:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 22:09:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 22:09:08]

Dry run: This issue passed the CQ dry run.

[tmartino, 2017-01-09 22:10:56]

The CQ bit was checked by tmartino@chromium.org

[commit-bot: I haz the power, 2017-01-09 22:11:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 22:20:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483999856034050,
"parent_rev": "f597e54c83b4da65da721b69e02ebd26ed181f92", "commit_rev":
"8890be37e55775f3fae49e71883c81a7c4ddd22a"}

[commit-bot: I haz the power, 2017-01-09 22:21:09]

Description was changed from

==========
Implementing Win 10 FRE visual design tweaks requested by first UX pass.

BUG=662175
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Implementing Win 10 FRE visual design tweaks requested by first UX pass.

BUG=662175
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2590253002
Cr-Commit-Position: refs/heads/master@{#442369}
Committed:
https://chromium.googlesource.com/chromium/src/+/8890be37e55775f3fae49e71883c...
==========

[commit-bot: I haz the power, 2017-01-09 22:21:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8890be37e55775f3fae49e71883c...