Add Edit/Create Address Dialog to MD Settings.

chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.js

Index: chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.js
diff --git a/chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.js b/chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.js
new file mode 100644
index 0000000000000000000000000000000000000000..dc194c44ee1e11f87460824b16a90f5a692ce164
--- /dev/null
+++ b/chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.js
@@ -0,0 +1,409 @@
+// Copyright 2016 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 'password-edit-dialog' is the dialog that allows showing a
+ *     saved password.

[michaelpg, 2016/06/22 00:25:13]

nit: no indent

[hcarmona, 2016/06/24 18:27:51]

Done.

+ */
+(function() {
+'use strict';
+
+Polymer({
+  is: 'settings-address-edit-dialog',
+
+  properties: {
+    /**
+     * @type {!chrome.autofillPrivate.AddressEntry}

[michaelpg, 2016/06/22 00:25:13]

opt nit here & below: /** @private {foo} */

[hcarmona, 2016/06/24 18:27:51]

Cool, done.

+     * @private
+     */
+    address_: Object,
+
+    /** @private */
+    title_: String,
+
+    /**
+     * @type {!Array<!chrome.autofillPrivate.CountryEntry>}
+     * @private
+     */
+    countries_: Array,
+
+    /**
+     * @type {string|undefined}
+     * @private
+     */
+    countryCode_: {
+      type: String,
+      observer: 'onUpdateCountryCode_',
+    },
+
+    /**
+     * @type {!settings.AddressEditDialog.AddressWrapper}
+     * @private
+     */
+    addressWrapper_: Object,
+
+    /** @private */
+    phoneNumber_: {
+      type: String,
+      observer: 'onUpdatePhoneNumber_',
+    },
+
+    /** @private */
+    email_: {
+      type: String,
+      observer: 'onUpdateEmail_',
+    },
+
+    /** @private */
+    canSave_: Boolean,
+  },
+
+  behaviors: [
+    I18nBehavior,
+  ],
+
+  /** @override */
+  ready: function() {
+    this.countryInfo =
+        settings.AddressEditDialog.CountryInformationHelperImpl.getInstance();
+
+    this.countryInfo.getCountryList(function(countryList) {
+      this.countries_ = countryList;
+    }.bind(this));
+  },
+
+  /**
+   * Opens the dialog to edit |address|
+   * @param {!chrome.autofillPrivate.AddressEntry} address
+   */
+  open: function(address) {
+    this.title_ =
+        this.i18n(address.guid ? 'editAddressTitle' : 'addAddressTitle');
+    this.address_ = address;
+
+    this.phoneNumber_ = address.phoneNumbers ? address.phoneNumbers[0] : '';
+    this.email_ = address.emailAddresses ? address.emailAddresses[0] : '';
+
+    if (this.countryCode_ == address.countryCode)
+      this.updateAddressWrapper_();
+    else
+      this.countryCode_ = address.countryCode;  // Updates the address wrapper.
+
+    // Open is called on the dialog after the address wrapper has been updated.
+  },
+
+  /**
+   * Returns a class to denote how long this entry is.
+   * @param {settings.AddressEditDialog.AddressSetting} setting
+   * @return {string}
+   */
+  long_: function(setting) {
+    return setting.isLongField ? 'long' : '';
+  },
+
+  /**
+   * Updates the wrapper that represents this address in the country's format.
+   * @private
+   */
+  updateAddressWrapper_: function() {
+    var self = this;
+    // Preserve the values when switching countries.
+    if (self.addressWrapper_)
+      self.addressWrapper_.writeToAddress();
+
+    // Default to the last country used if no country code is provided.
+    var countryCode = self.countryCode_ || self.countries_[0].countryCode;
+    self.countryInfo.getAddressFormat(countryCode, function(format) {
+      self.addressWrapper_ = settings.AddressEditDialog.AddressWrapper.create(
+          self.address_, format);
+      // Flush dom before resize and savability updates.
+      Polymer.dom.flush();
+
+/**
+ * TODO(hcarmona): Fix closure compiler to better understand |SettingsDialog|.
+ * @suppress {missingProperties}
+ */
+(function() {

[michaelpg, 2016/06/22 00:25:13]

indent?

[hcarmona, 2016/06/24 18:27:50]

Left this un-indented so it stands out. I can indent if it makes more
sense, but it's supposed to be analogous to an #ifdef.

[michaelpg, 2016/06/27 21:29:18]

Acknowledged.

+      self.$.dialog.notifyResize();
+
+      self.updateCanSave_();
+
+      self.fire('on-update-address-wrapper');  // For easier testing.
+
+      if (!self.$.dialog.opened)
+        self.$.dialog.open();
+})();
+    });
+  },
+
+  updateCanSave_: function() {
+    var inputs = this.$.dialog.querySelectorAll('.address-column');
+
+    for (var i = 0; i < inputs.length; ++i) {
+      if (inputs[i].value) {
+        this.canSave_ = true;
+        this.fire('on-update-can-save');  // For easier testing.
+        return;
+      }
+    }
+
+    this.canSave_ = false;
+    this.fire('on-update-can-save');  // For easier testing.
+  },
+
+  /**
+   * @param {!chrome.autofillPrivate.CountryEntry} country
+   * @return {string}
+   * @private
+   */
+  getCode_: function(country) {
+    return country.countryCode || 'SPACER';
+  },
+
+  /**
+   * @param {!chrome.autofillPrivate.CountryEntry} country
+   * @return {string}
+   * @private
+   */
+  getName_: function(country) {
+    return country.name || '------';
+  },
+
+  /**
+   * @param {!chrome.autofillPrivate.CountryEntry} country
+   * @return {boolean}
+   * @private
+   */
+  isDivision_: function(country) {
+    return !country.countryCode;
+  },
+
+  /**
+   * Handler for tapping the save button.
+   * @private
+   */
+  onSaveButtonTap_: function() {
+    this.addressWrapper_.writeToAddress();
+
+    // Set a default country if none is set.
+    if (!this.address_.countryCode)
+      this.address_.countryCode = this.countries_[0].countryCode;
+
+    this.fire('save-address', this.address_);
+    this.$.dialog.close();
+  },
+
+  /**
+   * Syncs the country code back to the address and rebuilds the address wrapper
+   * for the new location.
+   * @param {!string} countryCode

[michaelpg, 2016/06/22 00:25:13]

string is non-null by default, no need to specify !

but... should this be {string|undefined} given the property can be undefined? 

[hcarmona, 2016/06/24 18:27:51]

Done.

+   * @private
+   */
+  onUpdateCountryCode_: function(countryCode) {
+    this.address_.countryCode = countryCode;
+    this.updateAddressWrapper_();
+  },
+
+  /**
+   * Syncs the phone number back to the address. An address can have only one
+   * phone number. Variants are deprecated.
+   * @param {string} phoneNumber
+   */
+  onUpdatePhoneNumber_: function(phoneNumber) {
+    if (this.address_)
+      this.address_.phoneNumbers = [phoneNumber];
+  },
+
+  /**
+   * Syncs the email back to the address. An address can have only one email.
+   * Variants are deprecated.
+   * @param {string} email
+   */
+  onUpdateEmail_: function(email) {
+    if (this.address_)
+      this.address_.emailAddresses = [email];
+  },
+});
+})();
+
+cr.define('settings.AddressEditDialog', function() {

[michaelpg, 2016/06/22 00:25:13]

maybe settings.address or something... settings.AddressEditDialog looks like a
constructor

[hcarmona, 2016/06/24 18:27:50]

Done.

+  /**
+   * Creates a wrapper against a single data member for an address.

[michaelpg, 2016/06/22 00:25:14]

Sorry, there's just too much abstraction here for me to understand the point. I
can't tell where the "value" for AddressSetting ultimately comes from; I see
that it can get the value from its address, and set that value back to its
address, but not how the value changes.

IMO, AddressSetting is doing too many things: creating closures around
getting/setting an address component, having its own value, describing what the
UI for the field its value corresponds to should look like. Can I suggest
breaking it out into:

* a "SetAddressField" Command class to set the value on the address:
https://sourcemaking.com/design_patterns/command
* an "AddressComponentUI" to describe the UI given an AddressComponent
(basically wrap AddressComponent plus isTextArea)

Then each AddressComponentUI or maybe just the dialog itself can create a
SetAddressField command.

[hcarmona, 2016/06/24 18:27:50]

Sounds good. Code looks simpler. Let me know what you think.

+   * @param {!chrome.autofillPrivate.AddressEntry} address
+   * @param {!chrome.autofillPrivate.AddressComponent} piece
+   * @constructor
+   */
+  function AddressSetting(address, piece) {
+    this.name = piece.fieldName;
+    this.isLongField = piece.isLongField;
+    this.value = AddressSetting.getValue_(address, piece.field);
+    this.setValue = AddressSetting.getSetter_(address, piece.field);
+    this.isTextArea =
+        piece.field == chrome.autofillPrivate.AddressField.ADDRESS_LINES;
+  }
+
+  AddressSetting.prototype = {
+    writeToAddress: function() {
+      this.setValue(this.value || '');
+    },
+  };
+
+  /**
+   * Gets the |field| value from the address.
+   * @param {!chrome.autofillPrivate.AddressEntry} address
+   * @param {string} field

[michaelpg, 2016/06/22 00:25:13]

c.aP.AddressField

[hcarmona, 2016/06/24 18:27:50]

Done.

+   * @return {string|undefined}
+   * @private
+   */
+  AddressSetting.getValue_ = function(address, field) {
+    switch (field) {
+      case chrome.autofillPrivate.AddressField.FULL_NAME:
+        return address.fullNames ? address.fullNames[0] : undefined;

[michaelpg, 2016/06/22 00:25:13]

[comment on] why 0?

[hcarmona, 2016/06/24 18:27:50]

Done.

+      case chrome.autofillPrivate.AddressField.COMPANY_NAME:
+        return address.companyName;
+      case chrome.autofillPrivate.AddressField.ADDRESS_LINES:
+        return address.addressLines;
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_1:
+        return address.addressLevel1;
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_2:
+        return address.addressLevel2;
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_3:
+        return address.addressLevel3;
+      case chrome.autofillPrivate.AddressField.POSTAL_CODE:
+        return address.postalCode;
+      case chrome.autofillPrivate.AddressField.SORTING_CODE:
+        return address.sortingCode;
+      case chrome.autofillPrivate.AddressField.COUNTRY_CODE:
+        return address.countryCode;
+      default:
+        return undefined;

[michaelpg, 2016/06/22 00:25:13]

assertNotReached?

[hcarmona, 2016/06/24 18:27:50]

Done.

+    }
+  };
+
+  /**
+   * Gets a setter for the |field| value from the address.
+   * @param {!chrome.autofillPrivate.AddressEntry} address
+   * @param {string} field

[michaelpg, 2016/06/22 00:25:13]

c.aP.AddressField

[hcarmona, 2016/06/24 18:27:50]

Done.

+   * @return {function(string):void|undefined}
+   * @private
+   */
+  AddressSetting.getSetter_ = function(address, field) {
+    switch (field) {
+      case chrome.autofillPrivate.AddressField.FULL_NAME:
+        return function(value) {
+          address.fullNames = address.fullNames || [];
+          address.fullNames[0] = value;
+        };
+      case chrome.autofillPrivate.AddressField.COMPANY_NAME:
+        return function(value) { address.companyName = value; };
+      case chrome.autofillPrivate.AddressField.ADDRESS_LINES:
+        return function(value) { address.addressLines = value; };
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_1:
+        return function(value) { address.addressLevel1 = value; };
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_2:
+        return function(value) { address.addressLevel2 = value; };
+      case chrome.autofillPrivate.AddressField.ADDRESS_LEVEL_3:
+        return function(value) { address.addressLevel3 = value; };
+      case chrome.autofillPrivate.AddressField.POSTAL_CODE:
+        return function(value) { address.postalCode = value; };
+      case chrome.autofillPrivate.AddressField.SORTING_CODE:
+        return function(value) { address.sortingCode = value; };
+      case chrome.autofillPrivate.AddressField.COUNTRY_CODE:
+        return function(value) { address.countryCode = value; };
+      default:
+        return undefined;
+    }
+  };
+
+  /** @constructor */
+  function AddressRow() {

[michaelpg, 2016/06/22 00:25:14]

what's an AddressRow? why does it have settings?

[hcarmona, 2016/06/24 18:27:50]

Acknowledged.

+    /** @type {!Array<!settings.AddressEditDialog.AddressSetting>} */
+    this.settings = [];
+  }
+
+  AddressRow.prototype = {
+    writeToAddress: function() {
+      this.settings.forEach(function(setting) {
+        setting.writeToAddress();
+      });
+    },
+  };
+
+  /** @constructor */
+  function AddressWrapper() {
+    /** @type {!Array<!settings.AddressEditDialog.AddressRow>} */

[michaelpg, 2016/06/22 00:25:13]

More descriptive name, please.

Either way, I feel like this is supposed to be a 1:1 mapping of our dialog,
which doesn't seem useful. If it's just a collection of descriptions of UI
components, does it need to be its own class?

[hcarmona, 2016/06/24 18:27:50]

Acknowledged.

+    this.rows = [];
+  }
+
+  AddressWrapper.prototype = {
+    writeToAddress: function() {
+      this.rows.forEach(function(row) {
+        row.writeToAddress();
+      });
+    },
+  };
+
+  AddressWrapper.create = function(address, format) {
+    var wrapper = new AddressWrapper();
+
+    format.components.forEach(function(component) {
+      var row = new AddressRow();
+      wrapper.rows.push(row);
+
+      component.row.forEach(function(piece) {
+        row.settings.push(new AddressSetting(address, piece));
+      });
+    });
+
+    return wrapper;
+  };
+
+  /** @interface */
+  function CountryInformationHelper() {}

[michaelpg, 2016/06/22 00:25:14]

s/InformationHelper/<something more descriptive>
https://big.corp.google.com/~jmcmaster/testing/2016/06/episode-431-identifier...

[hcarmona, 2016/06/24 18:27:51]

Done.

+  CountryInformationHelper.prototype = {
+    /**
+     * Gets the list of available countries with their country code. List should
+     * have a space between the default country and an alphabetized list of

[michaelpg, 2016/06/22 00:25:13]

I don't understand this comment given that CountryEntry is just a single name
and two-letter code.

[hcarmona, 2016/06/24 18:27:50]

Attempted to make comment clearer. WDYT?

+     * countries. Country names should be internationalized.

[michaelpg, 2016/06/22 00:25:13]

the IDL already specifies it's i18n'ed

[hcarmona, 2016/06/24 18:27:51]

Done.

+     * @param {function(!Array<!chrome.autofillPrivate.CountryEntry>)} callback

[michaelpg, 2016/06/22 00:25:13]

maybe @return a Promise<!Array...> instead? getFoo implies to me you're
returning something.

[hcarmona, 2016/06/24 18:27:50]

Done.

+     */
+    getCountryList: assertNotReached,
+
+    /**
+     * Gets the address format for a given country code.
+     * @param {string} countryCode
+     * @param {function(!chrome.autofillPrivate.AddressComponents)} callback
+     */
+    getAddressFormat: assertNotReached,
+  };
+
+  /**
+   * Default implementation. Override for testing.
+   * @implements {settings.AddressEditDialog.CountryInformationHelper}
+   * @constructor
+   */
+  function CountryInformationHelperImpl() {}
+  cr.addSingletonGetter(CountryInformationHelperImpl);
+  CountryInformationHelperImpl.prototype = {
+    __proto__: CountryInformationHelper,
+
+    /** @override */
+    getCountryList: function(callback) {
+      chrome.autofillPrivate.getCountryList(callback);
+    },
+
+    /** @override */
+    getAddressFormat: function(countryCode, callback) {
+      chrome.autofillPrivate.getAddressComponents(countryCode || '', callback);

[michaelpg, 2016/06/22 00:25:14]

countryCode is {string}, when is it falsy other than ''

[hcarmona, 2016/06/24 18:27:51]

Done.

+    },
+  };
+
+  return {
+    AddressSetting: AddressSetting,
+    AddressRow: AddressRow,
+    AddressWrapper: AddressWrapper,
+    CountryInformationHelper: CountryInformationHelper,
+    CountryInformationHelperImpl: CountryInformationHelperImpl,
+  };
+});