MD Settings: Internet: Cleanup JS

chrome/browser/resources/settings/internet_page/network_apnlist.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 and modifying a list of cellular
  * access points.
  */
 Polymer({
   is: 'network-apnlist',
 
   properties: {
     /**
      * The current set of properties for the network matching |guid|.
      * @type {!CrOnc.NetworkProperties|undefined}
      */
     networkProperties: {
       type: Object,
       observer: 'networkPropertiesChanged_',
     },
 
-    /**
-     * The CrOnc.APNProperties.AccessPointName value of the selected APN.
-     */
-    selectedApn: {
+    /** The CrOnc.APNProperties.AccessPointName value of the selected APN. */
+    selectedApn_: {
       type: String,
       value: '',
     },
 
     /**
      * Selectable list of APN dictionaries for the UI. Includes an entry
      * corresponding to |otherApn| (see below).
      * @type {!Array<!CrOnc.APNProperties>}
      */
-    apnSelectList: {
+    apnSelectList_: {
       type: Array,
       value: function() {
         return [];
       }
     },
 
     /**
      * The user settable properties for a new ('other') APN. The values for
      * AccessPointName, Username, and Password will be set to the currently
      * active APN if it does not match an existing list entry.
      * @type {CrOnc.APNProperties|undefined}
      */
-    otherApn: {
+    otherApn_: {
       type: Object,
     },
 
     /**
      * Array of property names to pass to the Other APN property list.
      * @type {!Array<string>}
      */
     otherApnFields_: {
       type: Array,
       value: function() {
@@ skipping 36 matching lines @@
       activeApn = /** @type {!CrOnc.APNProperties|undefined} */ (
           CrOnc.getSimpleActiveProperties(apn));
     } else if (cellular.LastGoodAPN && cellular.LastGoodAPN.AccessPointName) {
       activeApn = cellular.LastGoodAPN;
     }
     this.setApnSelectList_(activeApn);
   },
 
   /**
    * Sets the list of selectable APNs for the UI. Appends an 'Other' entry
-   * (see comments for |otherApn| above).
+   * (see comments for |otherApn_| above).
    * @param {CrOnc.APNProperties|undefined} activeApn The currently active APN
    *     properties.
    * @private
    */
   setApnSelectList_: function(activeApn) {
     // Copy the list of APNs from this.networkProperties.
     var result = this.getApnList_().slice();
 
     // Test whether |activeApn| is in the current APN list in networkProperties.
     var activeApnInList = activeApn && result.some(function(a) {
       return a.AccessPointName == activeApn.AccessPointName;
     });
 
     // If |activeApn| is specified and not in the list, use the active
     // properties for 'other'. Otherwise use any existing 'other' properties.
     var otherApnProperties =
-        (activeApn && !activeApnInList) ? activeApn : this.otherApn;
+        (activeApn && !activeApnInList) ? activeApn : this.otherApn_;
     var otherApn = this.createApnObject_(otherApnProperties);
 
     // Always use 'Other' for the name of custom APN entries (the name does
     // not get saved).
     otherApn.Name = 'Other';
 
     // If no 'active' or 'other' AccessPointName was provided, use the default.
     otherApn.AccessPointName =
         otherApn.AccessPointName || this.DefaultAccessPointName;
 
     // Save the 'other' properties.
-    this.otherApn = otherApn;
+    this.otherApn_ = otherApn;
 
     // Append 'other' to the end of the list of APNs.
     result.push(otherApn);
 
-    this.apnSelectList = result;
-    this.selectedApn =
+    this.apnSelectList_ = result;
+    this.selectedApn_ =
         (activeApn && activeApn.AccessPointName) || otherApn.AccessPointName;
     // We need to flush the DOM here, otherwise the paper-dropdown-menu-light
     // will not update to correctly display the selected AccessPointName.
     Polymer.dom.flush();
   },
 
   /**
    * @param {!CrOnc.APNProperties|undefined=} apnProperties
    * @return {!CrOnc.APNProperties} A new APN object with properties from
    *     |apnProperties| if provided.
@@ skipping 17 matching lines @@
     /** @type {!chrome.networkingPrivate.ManagedAPNList|undefined} */ var
         apnlist = this.networkProperties.Cellular.APNList;
     if (!apnlist)
       return [];
     return /** @type {!Array<!CrOnc.APNProperties>} */ (
         CrOnc.getActiveValue(apnlist));
   },
 
   /**
    * Event triggered when the selectApn selection changes.
-   * @param {!{detail: !{selected: string}}} e
+   * @param {!{detail: !{selected: string}}} event

[Dan Beam, 2016/08/31 21:59:47]

nit: CustomEvent (and then cast detail to {selected: string})

    * @private
    */
-  onSelectApnChange_: function(e) {
-    var selectedApn = e.detail.selected;
+  onSelectApnChange_: function(event) {
+    var selectedApn = event.detail.selected;
     // When selecting 'Other', don't set a change event unless a valid
     // non-default value has been set for Other.
-    if (this.isOtherSelected_(this.networkProperties, selectedApn) &&
-        (!this.otherApn || !this.otherApn.AccessPointName ||
-         this.otherApn.AccessPointName == this.DefaultAccessPointName)) {
+    if (this.isOtherSelected_(selectedApn) &&
+        (!this.otherApn_ || !this.otherApn_.AccessPointName ||
+         this.otherApn_.AccessPointName == this.DefaultAccessPointName)) {
       return;
     }
     this.sendApnChange_(selectedApn);
   },
 
   /**
    * Event triggered when any 'Other' APN network property changes.
    * @param {!{detail: {field: string, value: string}}} event
    * @private
    */
   onOtherApnChange_: function(event) {
     this.set('otherApn.' + event.detail.field, event.detail.value);
     // Don't send a change event for 'Other' until the 'Save' button is tapped.
   },
 
   /**
    * Event triggered when the Other APN 'Save' button is tapped.
    * @param {Event} event
    * @private
    */
   onSaveOtherTap_: function(event) {
-    this.sendApnChange_(this.selectedApn);
+    this.sendApnChange_(this.selectedApn_);
   },
 
   /**
    * Send the apn-change event.
    * @param {string} selectedApn
    * @private
    */
   sendApnChange_: function(selectedApn) {
     var apnList = this.getApnList_();
     var apn = this.findApnInList(apnList, selectedApn);
     if (apn == undefined) {
       apn = this.createApnObject_();
-      if (this.otherApn) {
-        apn.AccessPointName = this.otherApn.AccessPointName;
-        apn.Username = this.otherApn.Username;
-        apn.Password = this.otherApn.Password;
+      if (this.otherApn_) {
+        apn.AccessPointName = this.otherApn_.AccessPointName;
+        apn.Username = this.otherApn_.Username;
+        apn.Password = this.otherApn_.Password;
       }
     }
     this.fire('apn-change', {field: 'APN', value: apn});
   },
 
   /**
-   * @param {!CrOnc.NetworkProperties|undefined} networkProperties
    * @param {string} selectedApn

[Dan Beam, 2016/08/31 21:59:47]

are you mixing this.* with @params?

[stevenjb, 2016/08/31 22:10:16]

I'm not sure I understand your question.

I am avoiding including properties as parameters (using this.foo instead). If a
property is a dependency and a non-property parameter is needed, I append the
properties at the end in the HTML. Does that seem reasonable?

    * @return {boolean} True if the 'other' APN is currently selected.
    * @private
    */
-  isOtherSelected_: function(networkProperties, selectedApn) {
-    if (!networkProperties || !networkProperties.Cellular)
+  isOtherSelected_: function(selectedApn) {
+    if (!this.networkProperties || !this.networkProperties.Cellular)
       return false;
     var apnList = this.getApnList_();
     var apn = this.findApnInList(apnList, selectedApn);
     return apn == undefined;
   },
 
   /**
    * @param {!CrOnc.APNProperties} apn
    * @return {string} The most descriptive name for the access point.
    * @private
@@ skipping 10 matching lines @@
    * @private
    */
   findApnInList: function(apnList, accessPointName) {
     for (let a of apnList) {
       if (a.AccessPointName == accessPointName)
         return a;
     }
     return undefined;
   }
 });