Implemented UI for the meetings controller out-of-box.

Implemented UI for the meetings controller out-of-box.

Implemented all the stages of the controller out-of-box.
Known issues:
* Most of the stages are not shown now during the real out-of-box, because C++
  part is not ready yet.
* Some transitions look bad/don't work.
* Spinner is wrong.

BUG=375191
TEST=manually

Committed: https://crrev.com/3f22ed18be6b033d8fa44693083517009fa3824b
Cr-Commit-Position: refs/heads/master@{#300017}

[dzhioev (left Google), 2014-10-10 13:39:13]

dzhioev@chromium.org changed reviewers:
+ merkulova@chromium.org, nkostylev@chromium.org

[dzhioev (left Google), 2014-10-10 13:39:14]

Hello, please review.
Here are the videos demonstrating how the UI works:
* Basic flow (no errors) http://goo.gl/eIGqFL
* Extended flow (with all the errors) http://goo.gl/ZYGGdn

Polymer is used extensively for the implementation, so you probably need to read
some Polymer docs to understand what is happening here.
Polymer site: https://www.polymer-project.org
Tutorial and getting started:
* https://www.polymer-project.org/docs/start/tutorial/intro.html
* https://www.polymer-project.org/docs/start/usingelements.html
* https://www.polymer-project.org/docs/start/creatingelements.html

For more details you can read all the articles in the "Polymer" section of the
Polymer site.

[dzhioev (left Google), 2014-10-10 13:53:02]

Information about core-* and paper-* elements can be found here
https://www.polymer-project.org/docs/elements/

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/indeterminate-progress.html
(right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/indeterminate-progress.html:23: :host {
About :host, :shadow and /deep/ you can read here
https://www.polymer-project.org/articles/styling-elements.html

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/indeterminate-progress.js (right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/indeterminate-progress.js:15: computed:
{
https://www.polymer-project.org/docs/polymer/polymer.html#computed-properties

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html
(right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:38:
<div flex horizontal end-justified layout center>
About these attributes you can read here
https://www.polymer-project.org/docs/polymer/layout-attrs.html

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js
(right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js:87:
observe: {
https://www.polymer-project.org/docs/polymer/polymer.html#observeblock

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js:111:
selectedDeviceChanged: function() {
https://www.polymer-project.org/docs/polymer/polymer.html#change-watchers

[dzhioev (left Google), 2014-10-10 14:27:24]

Here are the mocks: https://folio.googleplex.com/hotrod/jay/OOBE/001-latest

[Nikita (slow), 2014-10-13 17:23:41]

First round of comments.

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/indeterminate-progress.js (right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/indeterminate-progress.js:8:
runnerColor: '#0f9d58',
Please double check that colors are from the palette.

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js
(right):

https://codereview.chromium.org/642313002/diff/180002/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js:18:
/** @const */ var ICON_COLORS = ['#F0B9CB', '#F0ACC3', '#F098B6', '#F084A9',
Should use some of these:
https://www.google.com/design/spec/style/color.html

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:79: <structure
name="IDR_CUSTOM_ELEMENTS_HTML"
file="resources\chromeos\login\custom_elements.html" flattenhtml="true"
type="chrome_html" variables="OOBE=oobe" expand_variables="true"/>
nit: variables not needed

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/controller_pairing_page.css
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:12:
font-weight: 500;
nit: how about having a single common set of rules for fonts?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:39: left:
48px;
RTL

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:87:
margin-left: -0.5em;
RTL

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:91:
margin-right: -0.5em;
RTL

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:95:
margin-right: 1em;
RTL

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:36:
style="color: {{device | colorByName}}"></core-icon>
nit: 4 spaces indent.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js
(left):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js:24:
/** @const */ var PAGE_DEVICES_DISCOVERY = 'devices-discovery';
Where these are gone? I guess these are not needed anymore?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/pairing_device_list.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:27:
margin-right: 1em;
-webkit-margin-end?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:38: left:
calc(40px + 1em);
Please add rtl support.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:52:
margin-right: 10px;
-webkit-margin-end?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/roboto_font.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/roboto_font.css:1: /* Copyright 2014 The
Chromium Authors. All rights reserved.
Please file a bug (or find existing one) and reference it here to place these
fonts as Chrome OS image resources.

[Nikita (slow), 2014-10-14 13:27:12]

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/indeterminate-progress.html
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/indeterminate-progress.html:16: with
<paper-progress> when it'll have an indeterminate mode.
nit: Please file a tracking bug for that.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:33:
<!-- TODO(dzhioev): replace 'core-item' with 'paper-item'. -->
nit: File tracking bug.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:59:
* big - if this class is set, slightly bigger font is used on page.
nit: Rename to bigFont?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:78:
backgroundColor="#87ceAC" runnerPortion="40">
nit: Please check color with palette.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:90:
<content select="paper-button"></content>
nit: Should you place this button(s) to another div for consistency?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:101:
<template id="help-button">
nit: Should id and class be different (help-button)?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:103:
label="{{'needHelp' | i18n}}"></paper-button>
nit: What would be the full string name here?
I think string name should reflect it's usage like helpButtonTxt

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:107:
<indeterminate-progress runnerColor="#0f9d58" backgroundColor="#87ceAC"
nit: Check color with palette.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:108:
runnerPortion="23"></indeterminate-progress>
nit: is this stub value?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:116:
<div class="title">{{'welcome' | i18n}}</div>
nit: Use more specific string name instead of just 'welcome'.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:126:
<paper-button label="{{'connect' | i18n}}" on-tap="{{userActed}}"
nit: connectBtn

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:132:
<div class="title">{{'troubleConnecting' | i18n}}</div>
nit: Unify all string names, some do have 'title' in them, some don't.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:134:
<paper-button label="{{'adviceGotIt' | i18n}}" on-tap="{{userActed}}"
nit: Btn suffix for string name.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:148:
<!-- TODO(dzhioev): Strings TBD. -->
nit: File tracking bug.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:183:
<paper-button label="{{'continue' | i18n}}" on-click="{{userActed}}"
nit: continueBtn (or similar)

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:209:
<paper-button label="{{'continueToHangouts' | i18n}}"
nit: continueToHangoutsBtn (or something similar).

[dzhioev (left Google), 2014-10-15 14:07:44]

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:79: <structure
name="IDR_CUSTOM_ELEMENTS_HTML"
file="resources\chromeos\login\custom_elements.html" flattenhtml="true"
type="chrome_html" variables="OOBE=oobe" expand_variables="true"/>
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> nit: variables not needed

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/controller_pairing_page.css
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:12:
font-weight: 500;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> nit: how about having a single common set of rules for fonts?

What do you mean by single set of rules?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:39: left:
48px;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> RTL

Created an issued and added TODO.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:87:
margin-left: -0.5em;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> RTL

ditto

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:91:
margin-right: -0.5em;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> RTL

ditto

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:95:
margin-right: 1em;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> RTL

ditto

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/indeterminate-progress.html
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/indeterminate-progress.html:16: with
<paper-progress> when it'll have an indeterminate mode.
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Please file a tracking bug for that.

Done, http://crbug.com/423363

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:33:
<!-- TODO(dzhioev): replace 'core-item' with 'paper-item'. -->
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: File tracking bug.

Done. http://crbug.com/423368

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:36:
style="color: {{device | colorByName}}"></core-icon>
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> nit: 4 spaces indent.

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:59:
* big - if this class is set, slightly bigger font is used on page.
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Rename to bigFont?

Done. big-font.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:78:
backgroundColor="#87ceAC" runnerPortion="40">
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Please check color with palette.

Checked. All right, it's Google green 200.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:90:
<content select="paper-button"></content>
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Should you place this button(s) to another div for consistency?

I don't think so. Second div will be completely useless.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:101:
<template id="help-button">
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Should id and class be different (help-button)?

No, why?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:103:
label="{{'needHelp' | i18n}}"></paper-button>
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: What would be the full string name here?
> I think string name should reflect it's usage like helpButtonTxt

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:107:
<indeterminate-progress runnerColor="#0f9d58" backgroundColor="#87ceAC"
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Check color with palette.

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:108:
runnerPortion="23"></indeterminate-progress>
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: is this stub value?

23? No, it's not.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:116:
<div class="title">{{'welcome' | i18n}}</div>
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Use more specific string name instead of just 'welcome'.

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:126:
<paper-button label="{{'connect' | i18n}}" on-tap="{{userActed}}"
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: connectBtn

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:132:
<div class="title">{{'troubleConnecting' | i18n}}</div>
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Unify all string names, some do have 'title' in them, some don't.

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:134:
<paper-button label="{{'adviceGotIt' | i18n}}" on-tap="{{userActed}}"
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: Btn suffix for string name.

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:148:
<!-- TODO(dzhioev): Strings TBD. -->
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: File tracking bug.

Done. http://crbug.com/423740

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:183:
<paper-button label="{{'continue' | i18n}}" on-click="{{userActed}}"
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: continueBtn (or similar)

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.html:209:
<paper-button label="{{'continueToHangouts' | i18n}}"
On 2014/10/14 13:27:12, Nikita Kostylev wrote:
> nit: continueToHangoutsBtn (or something similar).

Done.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js
(left):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_screen_controller_pairing.js:24:
/** @const */ var PAGE_DEVICES_DISCOVERY = 'devices-discovery';
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> Where these are gone? I guess these are not needed anymore?

Yes, we use these identifiers directly in HTML.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/pairing_device_list.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:27:
margin-right: 1em;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> -webkit-margin-end.
Created issue and added TODO.

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:38: left:
calc(40px + 1em);
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> Please add rtl support.

ditto

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:52:
margin-right: 10px;
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> -webkit-margin-end?

ditto

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/roboto_font.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/roboto_font.css:1: /* Copyright 2014 The
Chromium Authors. All rights reserved.
On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> Please file a bug (or find existing one) and reference it here to place these
> fonts as Chrome OS image resources.

Done. http://crbug.com/409047

[Nikita (slow), 2014-10-15 14:38:38]

lgtm

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/controller_pairing_page.css
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:12:
font-weight: 500;
On 2014/10/15 14:07:42, dzhioev wrote:
> On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > nit: how about having a single common set of rules for fonts?
> 
> What do you mean by single set of rules? 

There're several css rules that use font-weight 300/500.

I was suggesting introducing css rules like
.font-light {
  font-weight: 300;
}

.font-medium {
  font-weight: 500;
}

Does this sound applicable here or not?

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/pairing_device_list.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:27:
margin-right: 1em;
On 2014/10/15 14:07:43, dzhioev wrote:
> On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > -webkit-margin-end.
> Created issue and added TODO.

Why not just replace margin-right right here? :)

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:52:
margin-right: 10px;
On 2014/10/15 14:07:44, dzhioev wrote:
> On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > -webkit-margin-end?
> 
> ditto

Why not just replace margin-right right here? :)

[merkulova, 2014-10-16 09:30:38]

lgtm with single nit

https://codereview.chromium.org/642313002/diff/390001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right):

https://codereview.chromium.org/642313002/diff/390001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:83: 
nit: remove the line

[dzhioev (left Google), 2014-10-16 21:27:59]

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/controller_pairing_page.css
(right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/controller_pairing_page.css:12:
font-weight: 500;
On 2014/10/15 14:38:38, Nikita Kostylev wrote:
> On 2014/10/15 14:07:42, dzhioev wrote:
> > On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > > nit: how about having a single common set of rules for fonts?
> > 
> > What do you mean by single set of rules? 
> 
> There're several css rules that use font-weight 300/500.
> 
> I was suggesting introducing css rules like
> .font-light {
>   font-weight: 300;
> }
> 
> .font-medium {
>   font-weight: 500;
> }
> 
> Does this sound applicable here or not?

I want to isolate all the page's decoration inside controller_pairing_page.css
But if we use suggested 'font-*' classes, we need to set them on elements
definition which happens outside of <controller-pairing-page>. If we do so, we
implicitly make an assumptions on how <controller-pairing-page> decoration
works, which is not good (for example it makes changes in
'controller-pairing-page' styling harder).

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/pairing_device_list.css (right):

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:27:
margin-right: 1em;
On 2014/10/15 14:38:38, Nikita Kostylev wrote:
> On 2014/10/15 14:07:43, dzhioev wrote:
> > On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > > -webkit-margin-end.
> > Created issue and added TODO.
> 
> Why not just replace margin-right right here? :)

Partial RTL support is worse than no RTL support =)

https://codereview.chromium.org/642313002/diff/310001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/pairing_device_list.css:52:
margin-right: 10px;
On 2014/10/15 14:38:38, Nikita Kostylev wrote:
> On 2014/10/15 14:07:44, dzhioev wrote:
> > On 2014/10/13 17:23:40, Nikita Kostylev wrote:
> > > -webkit-margin-end?
> > 
> > ditto
> 
> Why not just replace margin-right right here? :)

ditto

[dzhioev (left Google), 2014-10-16 21:43:37]

dzhioev@chromium.org changed reviewers:
+ jhawkins@chromium.org

[dzhioev (left Google), 2014-10-16 21:44:32]

+James Hawkins for chrome/browser/browser_resources.grd review.

[James Hawkins, 2014-10-16 21:45:37]

lgtm

[dzhioev (left Google), 2014-10-16 21:48:54]

The CQ bit was checked by dzhioev@chromium.org

[commit-bot: I haz the power, 2014-10-16 21:53:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/642313002/410001

[commit-bot: I haz the power, 2014-10-16 23:59:32]

Committed patchset #5 (id:410001)

[commit-bot: I haz the power, 2014-10-17 00:00:00]

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