Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection. r=jaws
It's no longer supported to change the selectedness of options directly since that state causes conflicts with the application's own state.
MozReview-Commit-ID: kioot4BXoX
--- a/toolkit/components/payments/res/components/rich-option.js
+++ b/toolkit/components/payments/res/components/rich-option.js
@@ -7,17 +7,22 @@
* <rich-option></rich-option>
* </rich-select>
*/
/* global ObservedPropertiesMixin */
/* exported RichOption */
class RichOption extends ObservedPropertiesMixin(HTMLElement) {
- static get observedAttributes() { return ["selected", "hidden"]; }
+ static get observedAttributes() {
+ return [
+ "selected",
+ "value",
+ ];
+ }
connectedCallback() {
this.render();
let richSelect = this.closest("rich-select");
if (richSelect && richSelect.render) {
richSelect.render();
}
@@ -52,36 +57,15 @@ class RichOption extends ObservedPropert
if (!this.disabled &&
event.which == 13 /* Enter */) {
for (let option of this.parentNode.children) {
option.selected = option == this;
}
}
}
- get selected() {
- return this.hasAttribute("selected");
- }
-
- set selected(value) {
- if (value) {
- let oldSelectedOptions = this.parentNode.querySelectorAll("[selected]");
- for (let option of oldSelectedOptions) {
- option.removeAttribute("selected");
- }
- this.setAttribute("selected", value);
- } else {
- this.removeAttribute("selected");
- }
- let richSelect = this.closest("rich-select");
- if (richSelect && richSelect.render) {
- richSelect.render();
- }
- return value;
- }
-
static _createElement(fragment, className) {
let element = document.createElement("span");
element.classList.add(className);
fragment.appendChild(element);
return element;
}
}
--- a/toolkit/components/payments/res/components/rich-select.js
+++ b/toolkit/components/payments/res/components/rich-select.js
@@ -1,20 +1,22 @@
/* 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/. */
+/* global ObservedPropertiesMixin */
+
/**
* <rich-select>
* <rich-option></rich-option>
* </rich-select>
+ *
+ * Note: The only supported way to change the selected option is via the
+ * `selectedOption` setter.
*/
-
-/* global ObservedPropertiesMixin */
-
class RichSelect extends ObservedPropertiesMixin(HTMLElement) {
static get observedAttributes() {
return [
"open",
"disabled",
"hidden",
];
}
@@ -43,18 +45,32 @@ class RichSelect extends ObservedPropert
get popupBox() {
return this.querySelector(":scope > .rich-select-popup-box");
}
get selectedOption() {
return this.popupBox.querySelector(":scope > [selected]");
}
- namedItem(name) {
- return this.popupBox.querySelector(`:scope > [name="${CSS.escape(name)}"]`);
+
+ /**
+ * This is the only supported method of changing the selected option. Do not
+ * manipulate the `selected` property or attribute on options directly.
+ * @param {HTMLOptionElement} option
+ */
+ set selectedOption(option) {
+ for (let child of this.popupBox.children) {
+ child.selected = child == option;
+ }
+
+ this.render();
+ }
+
+ getOptionByValue(value) {
+ return this.popupBox.querySelector(`:scope > [value="${CSS.escape(value)}"]`);
}
handleEvent(event) {
switch (event.type) {
case "blur": {
this.onBlur(event);
break;
}
@@ -112,16 +128,19 @@ class RichSelect extends ObservedPropert
let aAttrs = a.constructor.observedAttributes;
let bAttrs = b.constructor.observedAttributes;
if (aAttrs.length != bAttrs.length) {
return false;
}
for (let aAttr of aAttrs) {
+ if (aAttr == "selected") {
+ continue;
+ }
if (a.getAttribute(aAttr) != b.getAttribute(aAttr)) {
return false;
}
}
return true;
}
@@ -140,32 +159,34 @@ class RichSelect extends ObservedPropert
for (let option of options) {
popupBox.appendChild(option);
}
let selectedChild;
for (let child of popupBox.children) {
if (child.selected) {
selectedChild = child;
+ break;
}
}
if (!selectedChild && popupBox.children.length) {
selectedChild = popupBox.children[0];
selectedChild.selected = true;
}
let selectedClone = this.querySelector(":scope > .rich-select-selected-clone");
if (!this._optionsAreEquivalent(selectedClone, selectedChild)) {
if (selectedClone) {
selectedClone.remove();
}
if (selectedChild) {
selectedClone = selectedChild.cloneNode(false);
selectedClone.removeAttribute("id");
+ selectedClone.removeAttribute("selected");
selectedClone.classList.add("rich-select-selected-clone");
selectedClone = this.appendChild(selectedClone);
}
}
}
}
customElements.define("rich-select", RichSelect);
--- a/toolkit/components/payments/res/containers/address-picker.js
+++ b/toolkit/components/payments/res/containers/address-picker.js
@@ -22,28 +22,43 @@ class AddressPicker extends PaymentState
this.appendChild(this.dropdown);
super.connectedCallback();
}
render(state) {
let {savedAddresses} = state;
let desiredOptions = [];
for (let [guid, address] of Object.entries(savedAddresses)) {
- let optionEl = this.dropdown.namedItem(guid);
+ let optionEl = this.dropdown.getOptionByValue(guid);
if (!optionEl) {
optionEl = document.createElement("address-option");
- optionEl.name = guid;
- optionEl.guid = guid;
+ optionEl.value = guid;
}
for (let [key, val] of Object.entries(address)) {
optionEl.setAttribute(key, val);
}
desiredOptions.push(optionEl);
}
let el = null;
while ((el = this.dropdown.popupBox.querySelector(":scope > address-option"))) {
el.remove();
}
- this.dropdown.popupBox.append(...desiredOptions);
+ for (let option of desiredOptions) {
+ this.dropdown.popupBox.appendChild(option);
+ }
+
+ // Update selectedness after the options are updated
+ let selectedAddressGUID = state[this.selectedStateKey];
+ let optionWithGUID = this.dropdown.getOptionByValue(selectedAddressGUID);
+ this.dropdown.selectedOption = optionWithGUID;
+
+ if (selectedAddressGUID && !optionWithGUID) {
+ throw new Error(`${this.selectedStateKey} option ${selectedAddressGUID}` +
+ `does not exist in options`);
+ }
+ }
+
+ get selectedStateKey() {
+ return this.getAttribute("selected-state-key");
}
}
customElements.define("address-picker", AddressPicker);
--- a/toolkit/components/payments/res/paymentRequest.xhtml
+++ b/toolkit/components/payments/res/paymentRequest.xhtml
@@ -40,18 +40,17 @@
</header>
<div id="main-container">
<section id="payment-summary">
<h1>Your Payment</h1>
<section>
<div><label>Shipping Address</label></div>
- <address-picker>
- </address-picker>
+ <address-picker selected-state-key="selectedShippingAddress"></address-picker>
</section>
<footer id="controls-container">
<button id="cancel">Cancel</button>
<button id="pay">Pay</button>
</footer>
</section>
<section id="order-details-overlay" hidden="hidden">
--- a/toolkit/components/payments/test/mochitest/test_rich_select.html
+++ b/toolkit/components/payments/test/mochitest/test_rich_select.html
@@ -159,17 +159,17 @@ add_task(async function test_clicking_on
"The selected clone email should be equivalent to the selected option 2");
is(selectedClone.getAttribute("name"), option2.getAttribute("name"),
"The selected clone name should be equivalent to the selected option 2");
});
add_task(async function test_changing_option_selected_affects_other_options() {
ok(option2.selected, "Option 2 should be selected from prior test");
- option1.selected = true;
+ select1.selectedOption = option1;
ok(!option2.selected, "Option 2 should no longer be selected after making option 1 selected");
ok(option1.hasAttribute("selected"), "Option 1 should now have selected attribute");
});
add_task(async function test_up_down_keys_change_selected_item() {
let openObserver = new MutationObserver(mutations => {
for (let mutation of mutations) {
ok(mutation.attributeName != "open", "the select should not open/close during this test");
@@ -275,17 +275,17 @@ add_task(async function test_selected_cl
let clonedOption = clonedOptions[0];
for (let attrName of AddressOption.observedAttributes) {
is(clonedOption.attributes[attrName] && clonedOption.attributes[attrName].value,
option1.attributes[attrName] && option1.attributes[attrName].value,
"attributes should have matching value; name=" + attrName);
}
- option2.selected = true;
+ select1.selectedOption = option2;
await asyncElementRendered();
clonedOptions = select1.querySelectorAll(".rich-select-selected-clone");
is(clonedOptions.length, 1, "there should only be one cloned option");
clonedOption = clonedOptions[0];
for (let attrName of AddressOption.observedAttributes) {
is(clonedOption.attributes[attrName] && clonedOption.attributes[attrName].value,