MD Settings: Internet: Use custom a11y labels for network toggle buttons

chrome/browser/resources/settings/internet_page/network_summary_item.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview Polymer element for displaying the network state for a specific
  * type and a list of networks for that type.
  */
 
 /** @typedef {chrome.networkingPrivate.DeviceStateProperties} */
 var DeviceStateProperties;
 
 Polymer({
   is: 'network-summary-item',
 
-  behaviors: [Polymer.IronA11yKeysBehavior],
+  behaviors: [Polymer.IronA11yKeysBehavior, I18nBehavior],

[hcarmona, 2017/02/23 00:52:21]

Another possibility is to use $i18nPolymer{...} in the HTML to pass in the
string values similar to this:
https://codereview.chromium.org/2693373002/diff/1/chrome/browser/resources/se...

[stevenjb, 2017/02/23 20:51:33]

There are currently 3 and eventually 4 strings (when we add a 'Tether' type)
that would need to be passed in, which seems like it would be pretty unwieldy in
the HTML for a relatively minor optimization.

[hcarmona, 2017/02/23 21:51:06]

Sounds good, no objections

 
   properties: {
     /**
      * Device state for the network type.
      * @type {!DeviceStateProperties|undefined}
      */
     deviceState: {
       type: Object,
       observer: 'deviceStateChanged_',
     },
 
     /**
      * Network state for the active network.
      * @type {!CrOnc.NetworkStateProperties|undefined}
      */
     activeNetworkState: Object,
 
-    /** String for a11y purposes. */
-    itemName: String,

[hcarmona, 2017/02/23 00:52:21]

itemName can also be removed from cr-network-list-item.

[stevenjb, 2017/02/23 20:51:33]

I saw that but decided that leaving itemName as a computed property seems fine.

-
     /**
      * List of all network state data for the network type.
      * @type {!Array<!CrOnc.NetworkStateProperties>}
      */
     networkStateList: {
       type: Array,
       value: function() {
         return [];
       },
     },
@@ skipping 184 matching lines @@
    * @param {!DeviceStateProperties} deviceState
    * @return {boolean}
    * @private
    */
   enableToggleIsEnabled_: function(deviceState) {
     return deviceState.State !=
         chrome.networkingPrivate.DeviceStateType.PROHIBITED;
   },
 
   /**
+   * @return {string}
+   * @private
+   */
+  getToggleA11yString_: function() {
+    switch (this.deviceState.Type) {
+      case CrOnc.Type.CELLULAR:
+        return this.i18n('internetToggleMobileA11yLabel');
+      case CrOnc.Type.WI_FI:
+        return this.i18n('internetToggleWiFiA11yLabel');
+      case CrOnc.Type.WI_MAX:
+        return this.i18n('internetToggleWiMAXA11yLabel');
+    }
+    return '';

[hcarmona, 2017/02/23 00:52:21]

Does it make sense to return "Unknown device enable" or to add an
assertNotReached() here?

[stevenjb, 2017/02/23 20:51:33]

I'd rather not added a string we don't plan to use, but I can early exit when we
expect the control to be hidden and add an assert.

[hcarmona, 2017/02/23 21:51:06]

Awesome, thanks :-)

+  },
+
+  /**
    * @return {boolean} Whether or not to show the UI to expand the list.
    * @private
    */
   expandIsVisible_: function() {
     if (!this.deviceIsEnabled_(this.deviceState))
       return false;
     var type = this.deviceState.Type;
     var minLength =
         (type == CrOnc.Type.WI_FI || type == CrOnc.Type.VPN) ? 1 : 2;
     return this.networkStateList.length >= minLength;
@@ skipping 64 matching lines @@
    */
   onDeviceEnabledTap_: function(event) {
     var deviceIsEnabled = this.deviceIsEnabled_(this.deviceState);
     var type = this.deviceState ? this.deviceState.Type : '';
     this.fire(
         'device-enabled-toggled', {enabled: !deviceIsEnabled, type: type});
     // Make sure this does not propagate to onDetailsTap_.
     event.stopPropagation();
   },
 });