Refresh ChromeVox options page CSS

Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox forward. Also tested in Classic mode (Compat).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2776293002
Cr-Commit-Position: refs/heads/master@{#460553}
Committed: https://chromium.googlesource.com/chromium/src/+/112077e34c0a22405ff8d0732cc1a059d896633c

[dmazzoni, 2017-03-27 20:22:41]

Description was changed from

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
==========

to

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

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

[dmazzoni, 2017-03-27 20:23:08]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[dmazzoni, 2017-03-27 20:59:10]

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

[commit-bot: I haz the power, 2017-03-27 20:59: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, 2017-03-27 22:22:32]

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

[commit-bot: I haz the power, 2017-03-27 22:22:33]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2017-03-28 01:04:26]

Please add
TEST=

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:107:
Columns:
Imo, rows and columns are very "dev" terms. Why not say "lines" and "line
length"?

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(right):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:84:
8-dot braille table:
Select an 8-dot braille table:

(you're mixing label phrasing). Here, there's a literal label where as
previously, we were using very descriptive labels.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:102:
Virtual Braille Display
Btw, I think this deserves a descriptive label e.g. shows a simulated multiline
braille display inside of the ChromeVox Panel.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js:298:
select8.setAttribute('tabIndex', 0);
They you test this with the tab key? It was a little tricky to make the hidden
element gone and unreachable via ChromeVox navigation. Furthermore, remember we
still are supporting Classic.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd:511:
Change the current 6 dot braille table by selecting an option from the list
below.
Why was this changed?

[dmazzoni, 2017-03-28 17:16:38]

Description was changed from

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

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

to

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-03-28 17:27:19]

Description was changed from

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward. Also tested in Classic mode.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-03-28 17:28:28]

Updated change description. I tested with Next and
Compat modes - note that because of the url we don't
actually ever get Classic.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:107:
Columns:
On 2017/03/28 01:04:26, David Tseng wrote:
> Imo, rows and columns are very "dev" terms. Why not say "lines" and "line
> length"?

Done.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(right):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:84:
8-dot braille table:
On 2017/03/28 01:04:26, David Tseng wrote:
> Select an 8-dot braille table:
> 
> (you're mixing label phrasing). Here, there's a literal label where as
> previously, we were using very descriptive labels.

Done.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:102:
Virtual Braille Display
On 2017/03/28 01:04:26, David Tseng wrote:
> Btw, I think this deserves a descriptive label e.g. shows a simulated
multiline
> braille display inside of the ChromeVox Panel.

Done.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js:298:
select8.setAttribute('tabIndex', 0);
On 2017/03/28 01:04:26, David Tseng wrote:
> They you test this with the tab key? It was a little tricky to make the hidden
> element gone and unreachable via ChromeVox navigation. Furthermore, remember
we
> still are supporting Classic.

I tested this with Tab and with ChromeVox navigation,
and with Classic also. Setting display:none on the ancestor
works fine, there shouldn't be any reason to also set the
tabIndex.

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd:511:
Change the current 6 dot braille table by selecting an option from the list
below.
On 2017/03/28 01:04:26, David Tseng wrote:
> Why was this changed?

I wanted it to be more concise because the visual
style of other settings in Chrome has the label
next to the control horizontally but this sentence
made it wrap.

Also, the phrasing "below" is now wrong, but also
I think it was confusing before if you navigated via
Tab since that became the control's label.

Happy to keep it an action rather than a noun, so
"Select a 6-dot braille table" works great.

[dmazzoni, 2017-03-28 17:28:42]

Description was changed from

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward. Also tested in Classic mode.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward. Also tested in Classic mode (Compat).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-03-28 18:48:52]

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
(left):

https://codereview.chromium.org/2776293002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd:511:
Change the current 6 dot braille table by selecting an option from the list
below.
On 2017/03/28 17:28:28, dmazzoni wrote:
> On 2017/03/28 01:04:26, David Tseng wrote:
> > Why was this changed?
> 
> I wanted it to be more concise because the visual
> style of other settings in Chrome has the label
> next to the control horizontally but this sentence
> made it wrap.
> 
> Also, the phrasing "below" is now wrong, but also
> I think it was confusing before if you navigated via
> Tab since that became the control's label.
> 
> Happy to keep it an action rather than a noun, so
> "Select a 6-dot braille table" works great.

Great, yes that's now consistent with the labels for the checkboxes.

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(right):

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:68:
Current voice:
Select current voice: (in keeping with the other labels). Also, you need to
change the value of the actual string resource that will replace this string
(i.e. "options_voices_description").

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:104:
<p>
Needs msgid.

[dmazzoni, 2017-03-29 06:33:56]

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html
(right):

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:68:
Current voice:
On 2017/03/28 18:48:51, David Tseng wrote:
> Select current voice: (in keeping with the other labels). Also, you need to
> change the value of the actual string resource that will replace this string
> (i.e. "options_voices_description").

Done.

https://codereview.chromium.org/2776293002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.html:104:
<p>
On 2017/03/28 18:48:51, David Tseng wrote:
> Needs msgid.

Done.

[dmazzoni, 2017-03-29 06:35:45]

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

[commit-bot: I haz the power, 2017-03-29 06:36:14]

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-03-29 07:33:59]

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

[commit-bot: I haz the power, 2017-03-29 07:34:00]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[David Tseng, 2017-03-29 20:41:04]

lgtm

[dmazzoni, 2017-03-29 21:16:22]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2017-03-29 21:17:38]

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-03-29 22:22:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490822182929490,
"parent_rev": "7566ff2a007b21cb152c279c4c7ed2d0013af96f", "commit_rev":
"112077e34c0a22405ff8d0732cc1a059d896633c"}

[commit-bot: I haz the power, 2017-03-29 22:24:41]

Description was changed from

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward. Also tested in Classic mode (Compat).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Refresh ChromeVox options page CSS

The style now roughly matches the new MD settings page, but without all
of the fancy MD effects.

BUG=none
TEST=Tried setting all ChromeVox options, navigating via Tab and via ChromeVox
forward. Also tested in Classic mode (Compat).
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2776293002
Cr-Commit-Position: refs/heads/master@{#460553}
Committed:
https://chromium.googlesource.com/chromium/src/+/112077e34c0a22405ff8d0732cc1...
==========

[commit-bot: I haz the power, 2017-03-29 22:24:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/112077e34c0a22405ff8d0732cc1...