ChromeOS OOBE: Implement network selection screen "Back" button.

ChromeOS OOBE: Implement network selection screen "Back" button.

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

Committed: https://crrev.com/89f8f02cb46e5043c20ef0f11814cb6c7c8650d0
Cr-Commit-Position: refs/heads/master@{#432385}

[Alexander Alekseev, 2016-11-12 03:50:55]

Description was changed from

==========
ChromeOS OOBE: Implement network selection screen "Back" button.

BUG=642400
==========

to

==========
ChromeOS OOBE: Implement network selection screen "Back" button.

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

[Alexander Alekseev, 2016-11-12 03:51:25]

alemate@chromium.org changed reviewers:
+ michaelpg@chromium.org

[Alexander Alekseev, 2016-11-12 03:51:25]

Please review.

[Alexander Alekseev, 2016-11-12 03:51:31]

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

[commit-bot: I haz the power, 2016-11-12 03:51:40]

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-11-12 04:57:59]

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

[commit-bot: I haz the power, 2016-11-12 04:58:00]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-11-14 21:25:19]

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_back_button.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_back_button.css:13: font:
var(--oobe-roboto-bold), sans-serif;
font-family

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_buttons.html (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_buttons.html:26: Note, that content
must have separate DOM node (i.e. a separete div).
"separate"

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_dialog_parameters.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_dialog_parameters.css:35: /* 
height: 100%;*/
don't keep commented-out code

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_eula.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_eula.css:39: padding: 0 24px 0
24px;
use shorthand: 0 24px;

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.css:22: padding: 0 24px 0
24px;
use shorthand: 0 24px;

(same for below entries)

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.css:43: 
remove blank lines

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:24: <g
transform="translate(1.000000, 0.000000)">
These icon definitions have a lot of extra junk: unnecessary transformations,
unused IDs, etc. Can you squash them? See
https://www.chromium.org/developers/updating-webui-for-material-design/using-...
for guidance.

[Alexander Alekseev, 2016-11-15 01:23:00]

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_back_button.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_back_button.css:13: font:
var(--oobe-roboto-bold), sans-serif;
On 2016/11/14 21:25:18, michaelpg wrote:
> font-family

--oobe-roboto-bold defines not only font family, but also font-weight.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_buttons.html (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_buttons.html:26: Note, that content
must have separate DOM node (i.e. a separete div).
On 2016/11/14 21:25:18, michaelpg wrote:
> "separate"

Done.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_dialog_parameters.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_dialog_parameters.css:35: /* 
height: 100%;*/
On 2016/11/14 21:25:18, michaelpg wrote:
> don't keep commented-out code

Done.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_eula.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_eula.css:39: padding: 0 24px 0
24px;
On 2016/11/14 21:25:18, michaelpg wrote:
> use shorthand: 0 24px;

Done.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.css:22: padding: 0 24px 0
24px;
On 2016/11/14 21:25:19, michaelpg wrote:
> use shorthand: 0 24px;
> 
> (same for below entries)

Done.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.css:43: 
On 2016/11/14 21:25:19, michaelpg wrote:
> remove blank lines

Done.

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:24: <g
transform="translate(1.000000, 0.000000)">
On 2016/11/14 21:25:19, michaelpg wrote:
> These icon definitions have a lot of extra junk: unnecessary transformations,
> unused IDs, etc. Can you squash them? See
>
https://www.chromium.org/developers/updating-webui-for-material-design/using-...
> for guidance.

Done.

[Alexander Alekseev, 2016-11-15 01:41:57]

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

[commit-bot: I haz the power, 2016-11-15 01:43:08]

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-11-15 03:50:38]

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

[commit-bot: I haz the power, 2016-11-15 03:50:38]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-11-16 01:50:29]

lgtm with notes

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_back_button.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_back_button.css:13: font:
var(--oobe-roboto-bold), sans-serif;
On 2016/11/15 01:23:00, Alexander Alekseev wrote:
> On 2016/11/14 21:25:18, michaelpg wrote:
> > font-family
> 
> --oobe-roboto-bold defines not only font family, but also font-weight. 

Then maybe the sans-serif option should go there?

It's confusing seeing it in this context.

https://codereview.chromium.org/2495153002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_welcome.css (right):

https://codereview.chromium.org/2495153002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_welcome.css:41: padding: 0 6px; /*
= 8px - 2px "back buttom border"*/
buttom => button?

what are the quotes for?

[Alexander Alekseev, 2016-11-16 02:16:52]

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_back_button.css (right):

https://codereview.chromium.org/2495153002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_back_button.css:13: font:
var(--oobe-roboto-bold), sans-serif;
On 2016/11/16 01:50:28, michaelpg wrote:
> On 2016/11/15 01:23:00, Alexander Alekseev wrote:
> > On 2016/11/14 21:25:18, michaelpg wrote:
> > > font-family
> > 
> > --oobe-roboto-bold defines not only font family, but also font-weight. 
> 
> Then maybe the sans-serif option should go there?
> 
> It's confusing seeing it in this context.

To me it is more confusing to store several font options in a variable that
actually substitutes string "Roboto Bold". So this property has value like
"Either 'Roboto Bold' or 'sans-serif'".

The problem is that we do not have roboto font aliases for different weights
(like "Roboto Medium") and have to use font-weight instead. So I wanted to still
be able to specify "Roboto Medium", "Roboto Bold", etc as a single identifier.
But this variable was not meant to be anything than a single font.

https://codereview.chromium.org/2495153002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_welcome.css (right):

https://codereview.chromium.org/2495153002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_welcome.css:41: padding: 0 6px; /*
= 8px - 2px "back buttom border"*/
On 2016/11/16 01:50:28, michaelpg wrote:
> buttom => button?
> 
> what are the quotes for?

To emphasize that these three words are a single term here. But it is probably
not needed as here are no other comments here.

Removed.

[Alexander Alekseev, 2016-11-16 02:17:15]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2016-11-16 02:17:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2495153002/#ps40001
(title: "Update after review")

[commit-bot: I haz the power, 2016-11-16 02:17:48]

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-11-16 03:29:49]

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

[commit-bot: I haz the power, 2016-11-16 03:29:50]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Alexander Alekseev, 2016-11-16 04:20:03]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2016-11-16 04:20:24]

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-11-16 06:22:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-16 06:26:27]

Description was changed from

==========
ChromeOS OOBE: Implement network selection screen "Back" button.

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

to

==========
ChromeOS OOBE: Implement network selection screen "Back" button.

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

Committed: https://crrev.com/89f8f02cb46e5043c20ef0f11814cb6c7c8650d0
Cr-Commit-Position: refs/heads/master@{#432385}
==========

[commit-bot: I haz the power, 2016-11-16 06:26:29]

Patchset 3 (id:??) landed as
https://crrev.com/89f8f02cb46e5043c20ef0f11814cb6c7c8650d0
Cr-Commit-Position: refs/heads/master@{#432385}