ChromeOS: Implement Accessibility Options screen of material design OOBE.

ChromeOS: Implement Accessibility Options screen of material design OOBE.

This CL adds Accessibility Options screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/d19df1cdee8e365d754eb8ba215db4e72a6b7463
Cr-Commit-Position: refs/heads/master@{#411267}

[Alexander Alekseev, 2016-08-09 04:30:20]

Description was changed from

==========
ChromeOS: Implement Accessibility Options screen of material design OOBE.

This CL adds Accessibility Options screen to material design OOBE.

BUG=604119
TEST=none
==========

to

==========
ChromeOS: Implement Accessibility Options screen of material design OOBE.

This CL adds Accessibility Options screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Alexander Alekseev, 2016-08-09 04:42:17]

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

[Alexander Alekseev, 2016-08-09 04:42:17]

This Cl doesn't have styles. I am going to correct styling later.

Please review.

[Alexander Alekseev, 2016-08-09 04:42:24]

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

[commit-bot: I haz the power, 2016-08-09 04:54:20]

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-08-09 05:03:05]

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

[commit-bot: I haz the power, 2016-08-09 05:03:06]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[michaelpg, 2016-08-09 20:33:09]

mostly minor things

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1647: <message
name="IDS_ACCESIBILITY_SECTION_TITLE" desc="Title of accessibility options
screen" meaning="A title of the dialog where user can control ChromeOS UI
accessibility (like large cursor or screen reader) options during initial device
setup.">
spelling: ACCESSIBILITY

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1650: <message
name="IDS_ACCESIBILITY_SECTION_HINT" desc="Under-title line on the accessibility
options screen" meaning="A hint to the user that these settings help to make
device more usable.">
same

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1651: Configure your device to be more usable.
These settings are more options are available once device has been set up.
fix 2nd sentence

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/custom_elements_oobe.html:17: <include
src="oobe_a11n_option.html">
accessibility => a11y

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:1: <!-- Copyright
2016 The Chromium Authors. All rights reserved.
no HTML copyright headers

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:5: <link
rel="import"
href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html">
import chrome://resources/html/polymer.html

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:26: 
nit: remove trailing blank line

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_a11n_option.js (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.js:6: is:
'oobe-a11y-option',
should match file name

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.js:14: value: false,
consider leaving off the default value -- it always gets set by preferences,
right? if so, no reason to make it initialize to false and then become true

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html:7: <link
rel="import" href="chrome://resources/polymer/v1_0/paper-item/paper-item.html">
alphabetize imports

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_types.js (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_types.js:44:
OobeTypes.A11yStatusSet;
nit: just A11yStatus. "Set" implies it's a set, but it's more like a map (or a
struct).

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:91: <h1
class="welcome-message" i18n-content="accessibilitySectionTitle"></h1>
wrap

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:92: <h1
class="welcome-message-hint" i18n-content="accessibilitySectionHint"></h1>
wrap

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:99: <span
class="checked-value">On</span>
i18n for all these?

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:133:
i18n-content="oobeOKButtonText">
4-space indent

[Alexander Alekseev, 2016-08-10 04:03:46]

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1647: <message
name="IDS_ACCESIBILITY_SECTION_TITLE" desc="Title of accessibility options
screen" meaning="A title of the dialog where user can control ChromeOS UI
accessibility (like large cursor or screen reader) options during initial device
setup.">
On 2016/08/09 20:33:08, michaelpg wrote:
> spelling: ACCESSIBILITY

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1650: <message
name="IDS_ACCESIBILITY_SECTION_HINT" desc="Under-title line on the accessibility
options screen" meaning="A hint to the user that these settings help to make
device more usable.">
On 2016/08/09 20:33:08, michaelpg wrote:
> same

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1651: Configure your device to be more usable.
These settings are more options are available once device has been set up.
On 2016/08/09 20:33:08, michaelpg wrote:
> fix 2nd sentence

This is a bit tricky, as it is the exact phrase that we have in mocks ;)

But I was told that this looks more like a Lorem ipsum (i.e. to be revised
later), so I just removed the second sentence.

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/custom_elements_oobe.html:17: <include
src="oobe_a11n_option.html">
On 2016/08/09 20:33:08, michaelpg wrote:
> accessibility => a11y

Done.

This was a hybrid of accessibility and internationalization ;)

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:1: <!-- Copyright
2016 The Chromium Authors. All rights reserved.
On 2016/08/09 20:33:08, michaelpg wrote:
> no HTML copyright headers

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:5: <link
rel="import"
href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html">
On 2016/08/09 20:33:08, michaelpg wrote:
> import chrome://resources/html/polymer.html

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.html:26: 
On 2016/08/09 20:33:08, michaelpg wrote:
> nit: remove trailing blank line

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_a11n_option.js (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.js:6: is:
'oobe-a11y-option',
On 2016/08/09 20:33:08, michaelpg wrote:
> should match file name

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_a11n_option.js:14: value: false,
On 2016/08/09 20:33:08, michaelpg wrote:
> consider leaving off the default value -- it always gets set by preferences,
> right? if so, no reason to make it initialize to false and then become true

Done.

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_i18n_dropdown.html:7: <link
rel="import" href="chrome://resources/polymer/v1_0/paper-item/paper-item.html">
On 2016/08/09 20:33:08, michaelpg wrote:
> alphabetize imports

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_types.js (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_types.js:44:
OobeTypes.A11yStatusSet;
On 2016/08/09 20:33:08, michaelpg wrote:
> nit: just A11yStatus. "Set" implies it's a set, but it's more like a map (or a
> struct).

Done.

I used "set" in its mathematical sense. Just a set of options classified as
"accessibility", each one having value.

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

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:91: <h1
class="welcome-message" i18n-content="accessibilitySectionTitle"></h1>
On 2016/08/09 20:33:08, michaelpg wrote:
> wrap

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:92: <h1
class="welcome-message-hint" i18n-content="accessibilitySectionHint"></h1>
On 2016/08/09 20:33:08, michaelpg wrote:
> wrap

Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:99: <span
class="checked-value">On</span>
On 2016/08/09 20:33:08, michaelpg wrote:
> i18n for all these?

Sorry, I missed this.
Done.

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_welcome.html:133:
i18n-content="oobeOKButtonText">
On 2016/08/09 20:33:08, michaelpg wrote:
> 4-space indent

Done.

[Alexander Alekseev, 2016-08-10 04:04:01]

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

[commit-bot: I haz the power, 2016-08-10 04:04:35]

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-08-10 04:11:37]

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

[commit-bot: I haz the power, 2016-08-10 04:11:38]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[michaelpg, 2016-08-10 15:58:38]

lgtm

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_types.js (right):

https://codereview.chromium.org/2223213003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_types.js:44:
OobeTypes.A11yStatusSet;
On 2016/08/10 04:03:46, Alexander Alekseev wrote:
> On 2016/08/09 20:33:08, michaelpg wrote:
> > nit: just A11yStatus. "Set" implies it's a set, but it's more like a map (or
a
> > struct).
> 
> Done.
> 
> I used "set" in its mathematical sense. Just a set of options classified as
> "accessibility", each one having value.

Totally understand. But Set (and Map, and {} (Object)) are actual JavaScript
data objects we use as types, so it's best to be precise.

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_a11y_option.js (right):

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_a11y_option.js:10: * If
polymer-toggle-button is checked.
nit: "paper-toggle-button" or "polymer toggle button".

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_a11y_option.js:21: value: null,
no default value needed

[Alexander Alekseev, 2016-08-10 23:02:27]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2016-08-10 23:02:27]

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/2223213003/#ps40001
(title: "Update after review.")

[Alexander Alekseev, 2016-08-10 23:02:37]

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/oobe_a11y_option.js (right):

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_a11y_option.js:10: * If
polymer-toggle-button is checked.
On 2016/08/10 15:58:37, michaelpg wrote:
> nit: "paper-toggle-button" or "polymer toggle button".

Done.

https://codereview.chromium.org/2223213003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/oobe_a11y_option.js:21: value: null,
On 2016/08/10 15:58:37, michaelpg wrote:
> no default value needed

Done.

[commit-bot: I haz the power, 2016-08-10 23:04:26]

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-08-10 23:12:54]

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

[commit-bot: I haz the power, 2016-08-10 23:12:55]

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

[Alexander Alekseev, 2016-08-10 23:39:51]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2016-08-10 23:39:51]

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/2223213003/#ps60001
(title: "Fix presubmit.")

[commit-bot: I haz the power, 2016-08-10 23:40:26]

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-08-11 01:57:08]

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

[commit-bot: I haz the power, 2016-08-11 01:57:09]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexander Alekseev, 2016-08-11 02:16:18]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2016-08-11 02:16:49]

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-08-11 04:39:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-11 05:08:13]

Description was changed from

==========
ChromeOS: Implement Accessibility Options screen of material design OOBE.

This CL adds Accessibility Options screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
ChromeOS: Implement Accessibility Options screen of material design OOBE.

This CL adds Accessibility Options screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/d19df1cdee8e365d754eb8ba215db4e72a6b7463
Cr-Commit-Position: refs/heads/master@{#411267}
==========

[commit-bot: I haz the power, 2016-08-11 05:08:14]

Patchset 4 (id:??) landed as
https://crrev.com/d19df1cdee8e365d754eb8ba215db4e72a6b7463
Cr-Commit-Position: refs/heads/master@{#411267}