MD Settings: Network a11y fixes

MD Settings: Network a11y fixes

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

Review-Url: https://codereview.chromium.org/2755643006
Cr-Commit-Position: refs/heads/master@{#460856}
Committed: https://chromium.googlesource.com/chromium/src/+/8bb0fef95b4f2af30e3670b9b3a85ce8c84e5d75

[stevenjb, 2017-03-16 23:05:46]

Description was changed from

==========
MD Settings: Network a11y fixes

BUG=699289
==========

to

==========
MD Settings: Network a11y fixes

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

[stevenjb, 2017-03-17 00:19:30]

stevenjb@chromium.org changed reviewers:
+ hcarmona@chromium.org

[stevenjb, 2017-03-17 00:19:31]

I finally went through and tested the network page a11y. I'm sure I missed some
edge cases, but it's a whole lot better now.

[hcarmona, 2017-03-17 18:33:04]

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/device_page.html (right):

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/device_page.html:64:
aria-label="$i18n{powerTitle}"></button>
nice catch!

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html
(right):

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html:35:
aria-label$="[[getDetailsA11yString_(item)]]">
what's the reason for not using item.Name? I ask b/c this is more complicated
and I'm wondering if there's performance implications. Is it very different than
the item.Name?

[stevenjb, 2017-03-17 20:56:13]

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/device_page.html (right):

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/device_page.html:64:
aria-label="$i18n{powerTitle}"></button>
On 2017/03/17 18:33:04, hcarmona wrote:
> nice catch!

Acknowledged.

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html
(right):

https://codereview.chromium.org/2755643006/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html:35:
aria-label$="[[getDetailsA11yString_(item)]]">
On 2017/03/17 18:33:04, hcarmona wrote:
> what's the reason for not using item.Name? I ask b/c this is more complicated
> and I'm wondering if there's performance implications. Is it very different
than
> the item.Name?

Since this is WiFi only, we could get away with item.Name. It is theoretically
possible that could be empty but should never happen in practice. Done.

[hcarmona, 2017-03-17 21:58:22]

https://codereview.chromium.org/2755643006/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html
(right):

https://codereview.chromium.org/2755643006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html:35:
aria-label$="[[getDetailsA11yString_(item)]]">
I think you missed one here.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:42:
aria-label$="[[getDetailsA11yString_(item)]]">
Maybe can be removed, see below.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.js (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.js:127:
getDetailsA11yString_: function(state) {
Maybe can be removed, see below.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.js (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.js:60: itemName: {
If you make a small change here, you won't need to compute similar text twice,
assuming that itemName and CrOnc.getNetworkName(state) (from above) return the
same or similar strings

change to:

ariaLabel: {
  type: String,
  notify: true,
  reflectToAttribute: true,
  computed: 'getItemName_(item, isListItem)',
}

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.js:204:
getDetailsA11yString_: function(state) {
Will the network name from the state be different than the network name from the
item?

[stevenjb, 2017-03-17 22:54:08]

PTAL

https://codereview.chromium.org/2755643006/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html
(right):

https://codereview.chromium.org/2755643006/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_known_networks_page.html:35:
aria-label$="[[getDetailsA11yString_(item)]]">
On 2017/03/17 21:58:22, hcarmona wrote:
> I think you missed one here.

Done.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:42:
aria-label$="[[getDetailsA11yString_(item)]]">
On 2017/03/17 21:58:22, hcarmona wrote:
> Maybe can be removed, see below.

Acknowledged.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.js (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.js:127:
getDetailsA11yString_: function(state) {
On 2017/03/17 21:58:22, hcarmona wrote:
> Maybe can be removed, see below.

Acknowledged.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.js (right):

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.js:60: itemName: {
On 2017/03/17 21:58:22, hcarmona wrote:
> If you make a small change here, you won't need to compute similar text twice,
> assuming that itemName and CrOnc.getNetworkName(state) (from above) return the
> same or similar strings
> 
> change to:
> 
> ariaLabel: {
>   type: String,
>   notify: true,
>   reflectToAttribute: true,
>   computed: 'getItemName_(item, isListItem)',
> }

Ah, nice. Done.

https://codereview.chromium.org/2755643006/diff/40001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.js:204:
getDetailsA11yString_: function(state) {
On 2017/03/17 21:58:22, hcarmona wrote:
> Will the network name from the state be different than the network name from
the
> item?

No, and this will cover OOBE usage which I didn't think about, so nice catch,
thanks!

[hcarmona, 2017-03-17 22:59:13]

LGTM

[stevenjb, 2017-03-30 18:36:50]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-03-30 18:36:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
hcarmona@chromium.org
Link to the patchset: https://codereview.chromium.org/2755643006/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-30 18:37:17]

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-30 19:52:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490899010933240,
"parent_rev": "9cde4e7a2ef89518ffdf7e5d227175788e4e6b0b", "commit_rev":
"8bb0fef95b4f2af30e3670b9b3a85ce8c84e5d75"}

[commit-bot: I haz the power, 2017-03-30 19:53:44]

Description was changed from

==========
MD Settings: Network a11y fixes

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

to

==========
MD Settings: Network a11y fixes

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

Review-Url: https://codereview.chromium.org/2755643006
Cr-Commit-Position: refs/heads/master@{#460856}
Committed:
https://chromium.googlesource.com/chromium/src/+/8bb0fef95b4f2af30e3670b9b3a8...
==========

[commit-bot: I haz the power, 2017-03-30 19:53:46]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/8bb0fef95b4f2af30e3670b9b3a8...