Bug 1475080 - Clean up the pickers on the payment summary page to match the spec. r?MattN draft
authorprathiksha <prathikshaprasadsuman@gmail.com>
Fri, 13 Jul 2018 17:08:07 -0700
changeset 820570 372bb7266a0068c68fb9186fee09971107507582
parent 820131 5a8107262015714d2907a85abc24c847ad9b32d2
push id116874
push userbmo:prathikshaprasadsuman@gmail.com
push dateThu, 19 Jul 2018 22:27:07 +0000
reviewersMattN
bugs1475080
milestone63.0a1
Bug 1475080 - Clean up the pickers on the payment summary page to match the spec. r?MattN MozReview-Commit-ID: FOyD7DkwjHk
browser/components/payments/res/components/rich-select.css
browser/components/payments/res/containers/address-picker.js
browser/components/payments/res/containers/payment-dialog.js
browser/components/payments/res/containers/payment-method-picker.js
browser/components/payments/res/containers/rich-picker.css
browser/components/payments/res/containers/rich-picker.js
browser/components/payments/res/containers/shipping-option-picker.js
browser/components/payments/res/paymentRequest.css
browser/components/payments/res/paymentRequest.xhtml
browser/components/payments/test/browser/browser_request_shipping.js
browser/components/payments/test/mochitest/test_payment_dialog.html
--- a/browser/components/payments/res/components/rich-select.css
+++ b/browser/components/payments/res/components/rich-select.css
@@ -1,14 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 rich-select {
-  display: inline-block;
+  border: 1px solid #0C0C0D33;
+  display: block;
+  margin: 14px 0;
   position: relative;
 }
 
 /* Focusing on the underlying select element outlines the outer
 rich-select wrapper making it appear like rich-select is focused. */
 rich-select:focus-within {
   outline: 1px dotted;
 }
--- a/browser/components/payments/res/containers/address-picker.js
+++ b/browser/components/payments/res/containers/address-picker.js
@@ -1,55 +1,35 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import AddressOption from "../components/address-option.js";
-import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
-import RichSelect from "../components/rich-select.js";
+import RichPicker from "./rich-picker.js";
 import paymentRequest from "../paymentRequest.js";
 
 /**
  * <address-picker></address-picker>
  * Container around add/edit links and <rich-select> with
  * <address-option> listening to savedAddresses & tempAddresses.
  */
 
-export default class AddressPicker extends PaymentStateSubscriberMixin(HTMLElement) {
+export default class AddressPicker extends RichPicker {
   static get observedAttributes() {
-    return ["address-fields"];
+    return RichPicker.observedAttributes.concat(["address-fields"]);
   }
 
   constructor() {
     super();
-    this.dropdown = new RichSelect();
-    this.dropdown.addEventListener("change", this);
     this.dropdown.setAttribute("option-type", "address-option");
-    this.addLink = document.createElement("a");
-    this.addLink.className = "add-link";
-    this.addLink.href = "javascript:void(0)";
-    this.addLink.textContent = this.dataset.addLinkLabel;
-    this.addLink.addEventListener("click", this);
-    this.editLink = document.createElement("a");
-    this.editLink.className = "edit-link";
-    this.editLink.href = "javascript:void(0)";
-    this.editLink.textContent = this.dataset.editLinkLabel;
-    this.editLink.addEventListener("click", this);
-  }
-
-  connectedCallback() {
-    this.appendChild(this.dropdown);
-    this.appendChild(this.addLink);
-    this.append(" ");
-    this.appendChild(this.editLink);
-    super.connectedCallback();
   }
 
   attributeChangedCallback(name, oldValue, newValue) {
-    if (oldValue !== newValue) {
+    super.attributeChangedCallback(name, oldValue, newValue);
+    if (name == "address-fields" && oldValue !== newValue) {
       this.render(this.requestStore.getState());
     }
   }
 
   /**
    * De-dupe and filter addresses for the given set of fields that will be visible
    *
    * @param {object} addresses
--- a/browser/components/payments/res/containers/payment-dialog.js
+++ b/browser/components/payments/res/containers/payment-dialog.js
@@ -40,17 +40,16 @@ export default class PaymentDialog exten
     this._payButton.addEventListener("click", this);
 
     this._viewAllButton = contents.querySelector("#view-all");
     this._viewAllButton.addEventListener("click", this);
 
     this._mainContainer = contents.getElementById("main-container");
     this._orderDetailsOverlay = contents.querySelector("#order-details-overlay");
 
-    this._shippingTypeLabel = contents.querySelector("#shipping-type-label");
     this._shippingAddressPicker = contents.querySelector("address-picker.shipping-related");
     this._shippingRelatedEls = contents.querySelectorAll(".shipping-related");
     this._payerRelatedEls = contents.querySelectorAll(".payer-related");
     this._payerAddressPicker = contents.querySelector("address-picker.payer-related");
 
     this._header = contents.querySelector("header");
 
     this._errorText = contents.querySelector("#error-text");
@@ -247,16 +246,18 @@ export default class PaymentDialog exten
     let additionalItems = this._getAdditionalDisplayItems(state);
     this._viewAllButton.hidden = !displayItems.length && !additionalItems.length;
 
     let shippingType = state.request.paymentOptions.shippingType || "shipping";
     this._shippingAddressPicker.dataset.addAddressTitle =
       this.dataset[shippingType + "AddressTitleAdd"];
     this._shippingAddressPicker.dataset.editAddressTitle =
       this.dataset[shippingType + "AddressTitleEdit"];
+    let addressPickerLabel = this._shippingAddressPicker.dataset[shippingType + "AddressLabel"];
+    this._shippingAddressPicker.setAttribute("label", addressPickerLabel);
 
     let totalItem = paymentRequest.getTotalItem(state);
     let totalAmountEl = this.querySelector("#total > currency-amount");
     totalAmountEl.value = totalItem.amount.value;
     totalAmountEl.currency = totalItem.amount.currency;
 
     // Show the total header on the address and basic card pages only during
     // on-boarding(FTU) and on the payment summary page.
@@ -289,19 +290,16 @@ export default class PaymentDialog exten
       }
       this._payerAddressPicker.setAttribute("address-fields", [...fieldNames].join(" "));
     } else {
       this._payerAddressPicker.removeAttribute("address-fields");
     }
     this._payerAddressPicker.dataset.addAddressTitle = this.dataset.payerTitleAdd;
     this._payerAddressPicker.dataset.editAddressTitle = this.dataset.payerTitleEdit;
 
-    this._shippingTypeLabel.querySelector("label").textContent =
-      this._shippingTypeLabel.dataset[shippingType + "AddressLabel"];
-
     this._renderPayButton(state);
 
     for (let page of this._mainContainer.querySelectorAll(":scope > .page")) {
       page.hidden = state.page.id != page.id;
     }
 
     let {
       changesPrevented,
--- a/browser/components/payments/res/containers/payment-method-picker.js
+++ b/browser/components/payments/res/containers/payment-method-picker.js
@@ -1,54 +1,35 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import BasicCardOption from "../components/basic-card-option.js";
-import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
-import RichSelect from "../components/rich-select.js";
+import RichPicker from "./rich-picker.js";
 import paymentRequest from "../paymentRequest.js";
 
 /**
  * <payment-method-picker></payment-method-picker>
  * Container around add/edit links and <rich-select> with
  * <basic-card-option> listening to savedBasicCards.
  */
 
-export default class PaymentMethodPicker extends PaymentStateSubscriberMixin(HTMLElement) {
+export default class PaymentMethodPicker extends RichPicker {
   constructor() {
     super();
-    this.dropdown = new RichSelect();
-    this.dropdown.addEventListener("change", this);
     this.dropdown.setAttribute("option-type", "basic-card-option");
-    this.spacerText = document.createTextNode(" ");
     this.securityCodeInput = document.createElement("input");
     this.securityCodeInput.autocomplete = "off";
     this.securityCodeInput.size = 3;
     this.securityCodeInput.addEventListener("change", this);
-    this.addLink = document.createElement("a");
-    this.addLink.className = "add-link";
-    this.addLink.href = "javascript:void(0)";
-    this.addLink.textContent = this.dataset.addLinkLabel;
-    this.addLink.addEventListener("click", this);
-    this.editLink = document.createElement("a");
-    this.editLink.className = "edit-link";
-    this.editLink.href = "javascript:void(0)";
-    this.editLink.textContent = this.dataset.editLinkLabel;
-    this.editLink.addEventListener("click", this);
   }
 
   connectedCallback() {
-    this.appendChild(this.dropdown);
-    this.appendChild(this.spacerText);
+    super.connectedCallback();
     this.appendChild(this.securityCodeInput);
-    this.appendChild(this.addLink);
-    this.append(" ");
-    this.appendChild(this.editLink);
-    super.connectedCallback();
   }
 
   render(state) {
     let basicCards = paymentRequest.getBasicCards(state);
     let desiredOptions = [];
     for (let [guid, basicCard] of Object.entries(basicCards)) {
       let optionEl = this.dropdown.getOptionByValue(guid);
       if (!optionEl) {
new file mode 100644
--- /dev/null
+++ b/browser/components/payments/res/containers/rich-picker.css
@@ -0,0 +1,44 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+.picker-label {
+  color: #0c0c0d;
+  font-weight: 700;
+  grid-area: label;
+}
+
+.add-link {
+  grid-area: add;
+}
+
+.edit-link {
+  grid-area: edit;
+  border-right: 1px solid #0C0C0D33;
+}
+
+payment-method-picker a,
+address-picker a {
+  float: right;
+  padding: 0 8px;
+}
+
+payment-method-picker {
+  display: grid;
+  grid-template-columns: 5fr 1fr auto auto;
+  grid-template-areas:
+    "label    spacer edit add"
+    "cc-number cvv   cvv cvv";
+}
+
+payment-method-picker rich-select {
+  grid-area: cc-number;
+}
+
+payment-method-picker input {
+  border: 1px solid #0C0C0D33;
+  border-left: none;
+  grid-area: cvv;
+  height: 30px;
+  margin: 14px 0; /* Has to be same as rich-select */
+}
new file mode 100644
--- /dev/null
+++ b/browser/components/payments/res/containers/rich-picker.js
@@ -0,0 +1,46 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
+import RichSelect from "../components/rich-select.js";
+
+export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement) {
+  static get observedAttributes() {
+    return ["label"];
+  }
+
+  constructor() {
+    super();
+    this.dropdown = new RichSelect();
+    this.dropdown.addEventListener("change", this);
+    this.dropdown.id = Math.floor(Math.random() * 1000000);
+    this.labelElement = document.createElement("label");
+    this.labelElement.className = "picker-label";
+    this.labelElement.setAttribute("for", this.dropdown.id);
+    this.addLink = document.createElement("a");
+    this.addLink.className = "add-link";
+    this.addLink.href = "javascript:void(0)";
+    this.addLink.textContent = this.dataset.addLinkLabel;
+    this.addLink.addEventListener("click", this);
+    this.editLink = document.createElement("a");
+    this.editLink.className = "edit-link";
+    this.editLink.href = "javascript:void(0)";
+    this.editLink.textContent = this.dataset.editLinkLabel;
+    this.editLink.addEventListener("click", this);
+  }
+
+  connectedCallback() {
+    this.appendChild(this.labelElement);
+    this.appendChild(this.addLink);
+    this.appendChild(this.editLink);
+    this.appendChild(this.dropdown);
+    super.connectedCallback();
+  }
+
+  attributeChangedCallback(name, oldValue, newValue) {
+    if (name == "label") {
+      this.labelElement.textContent = newValue;
+    }
+  }
+}
--- a/browser/components/payments/res/containers/shipping-option-picker.js
+++ b/browser/components/payments/res/containers/shipping-option-picker.js
@@ -1,36 +1,31 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import PaymentStateSubscriberMixin from "../mixins/PaymentStateSubscriberMixin.js";
-import RichSelect from "../components/rich-select.js";
+import RichPicker from "./rich-picker.js";
 import ShippingOption from "../components/shipping-option.js";
 
 /**
  * <shipping-option-picker></shipping-option-picker>
  * Container around <rich-select> with
  * <option> listening to shippingOptions.
  */
 
-export default class ShippingOptionPicker extends PaymentStateSubscriberMixin(HTMLElement) {
+export default class ShippingOptionPicker extends RichPicker {
   constructor() {
     super();
-    this.dropdown = new RichSelect();
-    this.dropdown.addEventListener("change", this);
     this.dropdown.setAttribute("option-type", "shipping-option");
   }
 
-  connectedCallback() {
-    this.appendChild(this.dropdown);
-    super.connectedCallback();
-  }
+  render(state) {
+    this.addLink.hidden = true;
+    this.editLink.hidden = true;
 
-  render(state) {
     let {shippingOptions} = state.request.paymentDetails;
     let desiredOptions = [];
     for (let option of shippingOptions) {
       let optionEl = this.dropdown.getOptionByValue(option.id);
       if (!optionEl) {
         optionEl = document.createElement("option");
         optionEl.value = option.id;
       }
--- a/browser/components/payments/res/paymentRequest.css
+++ b/browser/components/payments/res/paymentRequest.css
@@ -50,16 +50,17 @@ payment-dialog > header {
   padding-top: 19px;
 }
 
 #main-container {
   display: flex;
   grid-area: main;
   position: relative;
   max-height: 100%;
+  padding-top: 18px;
 }
 
 .page {
   display: flex;
   flex-direction: column;
   height: 100%;
   position: relative;
   width: 100%;
--- a/browser/components/payments/res/paymentRequest.xhtml
+++ b/browser/components/payments/res/paymentRequest.xhtml
@@ -68,16 +68,17 @@
   <link rel="stylesheet" href="components/rich-select.css"/>
   <link rel="stylesheet" href="components/address-option.css"/>
   <link rel="stylesheet" href="components/basic-card-option.css"/>
   <link rel="stylesheet" href="components/shipping-option.css"/>
   <link rel="stylesheet" href="components/payment-details-item.css"/>
   <link rel="stylesheet" href="containers/address-form.css"/>
   <link rel="stylesheet" href="containers/basic-card-form.css"/>
   <link rel="stylesheet" href="containers/order-details.css"/>
+  <link rel="stylesheet" href="containers/rich-picker.css"/>
 
   <script src="unprivileged-fallbacks.js"></script>
 
   <script src="formautofill/autofillEditForms.js"></script>
 
   <script type="module" src="containers/payment-dialog.js"></script>
   <script type="module" src="paymentRequest.js"></script>
 
@@ -91,39 +92,34 @@
         <button id="view-all" class="closed">&viewAllItems;</button>
       </div>
     </header>
 
     <div id="main-container">
       <payment-request-page id="payment-summary">
         <div class="page-body">
           <div id="error-text"></div>
-
-          <div class="shipping-related"
-               id="shipping-type-label"
-               data-shipping-address-label="&shippingAddressLabel;"
-               data-delivery-address-label="&deliveryAddressLabel;"
-               data-persist-checkbox-label="&addressPage.persistCheckbox.label;"
-               data-pickup-address-label="&pickupAddressLabel;"><label></label></div>
           <address-picker class="shipping-related"
                           data-add-link-label="&address.addLink.label;"
                           data-edit-link-label="&address.editLink.label;"
+                          data-shipping-address-label="&shippingAddressLabel;"
+                          data-delivery-address-label="&deliveryAddressLabel;"
+                          data-pickup-address-label="&pickupAddressLabel;"
                           selected-state-key="selectedShippingAddress"></address-picker>
 
-          <div class="shipping-related"><label>&shippingOptionsLabel;</label></div>
-          <shipping-option-picker class="shipping-related"></shipping-option-picker>
+          <shipping-option-picker class="shipping-related"
+                                  label="&shippingOptionsLabel;"></shipping-option-picker>
 
-          <div><label>&paymentMethodsLabel;</label></div>
           <payment-method-picker selected-state-key="selectedPaymentCard"
                                  data-add-link-label="&basicCard.addLink.label;"
-                                 data-edit-link-label="&basicCard.editLink.label;">
+                                 data-edit-link-label="&basicCard.editLink.label;"
+                                 label="&paymentMethodsLabel;">
           </payment-method-picker>
-
-          <div class="payer-related"><label>&payerLabel;</label></div>
           <address-picker class="payer-related"
+                          label="&payerLabel;"
                           data-add-link-label="&payer.addLink.label;"
                           data-edit-link-label="&payer.editLink.label;"
                           selected-state-key="selectedPayerAddress"></address-picker>
           <div id="error-text"></div>
         </div>
 
         <footer>
           <button id="cancel">&cancelPaymentButton.label;</button>
--- a/browser/components/payments/test/browser/browser_request_shipping.js
+++ b/browser/components/payments/test/browser/browser_request_shipping.js
@@ -30,20 +30,20 @@ add_task(async function test_request_shi
         }
       );
 
       await spawnPaymentDialogTask(frame, async ([aShippingKey, aShippingString]) => {
         let shippingOptionPicker = content.document.querySelector("shipping-option-picker");
         ok(content.isVisible(shippingOptionPicker),
            "shipping-option-picker should be visible");
         const addressSelector = "address-picker[selected-state-key='selectedShippingAddress']";
-        let shippingAddress = content.document.querySelector(addressSelector);
-        ok(content.isVisible(shippingAddress),
+        let shippingAddressPicker = content.document.querySelector(addressSelector);
+        ok(content.isVisible(shippingAddressPicker),
            "shipping address picker should be visible");
-        let shippingOption = content.document.querySelector("#shipping-type-label");
+        let shippingOption = shippingAddressPicker.querySelector("label");
         is(shippingOption.textContent, aShippingString,
            "Label should be match shipping type: " + aShippingKey);
       }, [shippingKey, shippingString]);
 
       spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
 
       await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
     }
--- a/browser/components/payments/test/mochitest/test_payment_dialog.html
+++ b/browser/components/payments/test/mochitest/test_payment_dialog.html
@@ -11,16 +11,17 @@ Test the payment-dialog custom element
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <script src="sinon-2.3.2.js"></script>
   <script src="payments_common.js"></script>
   <script src="../../res/vendor/custom-elements.min.js"></script>
   <script src="../../res/unprivileged-fallbacks.js"></script>
 
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <link rel="stylesheet" type="text/css" href="../../res/paymentRequest.css"/>
+  <link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
 </head>
 <body>
   <p id="display">
     <iframe id="templateFrame" src="../../res/paymentRequest.xhtml" width="0" height="0"></iframe>
   </p>
 <div id="content" style="display: none">
 
 </div>