1427950 - Implement a multi-line PaymentRequest shipping address picker

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

Using the data from bug 1387221 and the widget from bug 1422164 we can implement a multi-line shipping address selector according to the UX spec.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Review commit: https://reviewboard.mozilla.org/r/214940/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214940/

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Clearing review since Jared told me it wasn't ready

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214940/diff/1-2/

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214940/diff/2-3/

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214940/diff/3-4/

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

All green try run, https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecc70ef43fcf160598eb601dff939aa28918b90f&selectedJob=158557433

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214940/diff/4-5/

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

New try run with the new test included, all green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f63df716080cdee85003d0521473f3885f4bd1a

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

https://reviewboard.mozilla.org/r/214940/#review221812

::: commit-message-5e046:1
(Diff revision 5)
> +Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker. r?MattN

Please change the commit message and bug summary to reflect the fact that this is now just about saving the selected address in state

::: toolkit/components/payments/res/components/rich-select.js:152
(Diff revision 5)
>      let selectedChild;
>      for (let child of popupBox.children) {
>        if (child.selected) {
> +        // Only allow one child to be selected at a time.
> +        if (selectedChild) {
> +          selectedChild.selected = false;
> +        }
>          selectedChild = child;
>        }
>      }
>      if (!selectedChild && popupBox.children.length) {
>        selectedChild = popupBox.children[0];
>        selectedChild.selected = true;
>      }

It seems like we have too many places dealing with option selectedness. I'm pretty sure it can be simplified by reducing local state and relying on the  state as the source of truth about which one or zero option is selected. I'll try out an approach locally.

::: toolkit/components/payments/res/containers/address-picker.js:19
(Diff revision 5)
>  
>  class AddressPicker extends PaymentStateSubscriberMixin(HTMLElement) {
>    constructor() {
>      super();
>      this.dropdown = document.createElement("rich-select");
> +    this.dropdown.picker = this;

This is leftover, right?

::: toolkit/components/payments/res/containers/address-picker.js:34
(Diff revision 5)
> +        let state = {};
> +        state[selectedKey] = select.querySelector("[selected]").guid;
> +        this.requestStore.setState(state);

This can all be done in one statement in a way that doesn't reduce readability IMO:
```js
this.requestStore.setState({
  [selectedKey]: select.querySelector("[selected]").guid,
});
```

::: toolkit/components/payments/res/containers/address-picker.js:54
(Diff revision 5)
> +      if (guid == state[this.selectedStateKey]) {
> +        optionEl.setAttribute("selected", "true");
> +      }

Why not have this handle unselecting the other options?  It seems like it could simplify things:
```js
optionEl.selected = guid == selectedGUID;

```

You should generally use setters, instead of setAttribute too.

::: toolkit/components/payments/res/containers/address-picker.js:54
(Diff revision 5)
>          optionEl.guid = guid;
>        }
>        for (let [key, val] of Object.entries(address)) {
>          optionEl.setAttribute(key, val);
>        }
> +      if (guid == state[this.selectedStateKey]) {

You could hoist this lookup outside the loop to help perf a bit:
```js
let selectedGUID = state[this.selectedStateKey];
```

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8944777 [details]
Bug 1427950 - Implement a multi-line PaymentRequest shipping address picker.

https://reviewboard.mozilla.org/r/214940/#review222132

I think since there are quite a few related issues that require refactoring things to make this work well it'll be better for me to write separate commits and get you to review them than to get you to fix issues in previous bugs that are now showing. We'll see what's left after my separate commits.

::: toolkit/components/payments/res/components/rich-select.js:96
(Diff revision 5)
>      } else if (event.key == "ArrowDown") {
>        let selectedOption = this.selectedOption;
>        let next = selectedOption.nextElementSibling;
>        if (next) {
>          next.selected = true;
>          selectedOption.selected = false;

We don't want to set the option to be selected until the user closes the popup (at least that's how <select> work on Windows 8 and macOS) and in the case of an address picker this may lead to a privacy leak and a bad experience since we would have to tell the merchant the user's address with every up and down key press plus we're supposed to prevent further changes while we're waiting for an update from the merchant. Which option is "selected" needs to be separated from what happens while the user uses the up+down keys (and also separate from any hover styling).

::: toolkit/components/payments/res/components/rich-select.js:155
(Diff revision 5)
> +        // Only allow one child to be selected at a time.
> +        if (selectedChild) {
> +          selectedChild.selected = false;
> +        }

This doesn't feel right to me since it seems like this will give rendering a rich-select a side-effect of changing an option.

Comment 11

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Add re-render button to PaymentRequest debugging panel.

This is useful to ensure that the UI state is properly represented in the store.
If the re-rendering causes visible changes then that means some state is not part
of the store or is not being rendered properly.

Review commit: https://reviewboard.mozilla.org/r/216590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216590/

Comment 12

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Have setStateFromParent handle removing deleted GUIDs from the "selected" keys.

Review commit: https://reviewboard.mozilla.org/r/216592/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216592/

Comment 13

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

It's no longer supported to change the selectedness of options directly since that state causes conflicts with the application's own state.

Review commit: https://reviewboard.mozilla.org/r/216594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216594/

Comment 14

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

This avoids the magic of the select automatically making the first option selected without the application state having a good way to know about this.

Review commit: https://reviewboard.mozilla.org/r/216596/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216596/

Comment 15

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Share common rich-option styles and make rich-select inline-block to improve layout.

Review commit: https://reviewboard.mozilla.org/r/216598/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216598/

Comment 16

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Create address and basic-card option cached children in the constructor and re-use them.

This avoids re-creating elements and getting references via querySelector. It's also a bit cleaner IMO.

Review commit: https://reviewboard.mozilla.org/r/216600/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216600/

Comment 17

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

Programmatic changes don't dispatch the event in order to avoid infinite loops.

Review commit: https://reviewboard.mozilla.org/r/216602/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216602/

Comment 18

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Use the <rich-select> 'change' event to update selectedShippingAddress state.

Review commit: https://reviewboard.mozilla.org/r/216604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216604/

Comment 19

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Only use the mutation observer for migrating new option children to the popup.

This reduces the side-effects of render (which should ideally be pure) and should improve performance. We already have the mutation observer anyways.

Review commit: https://reviewboard.mozilla.org/r/216606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216606/

Comment 20

[Matthew N. [:MattN]]

Attachment: Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

Review commit: https://reviewboard.mozilla.org/r/216608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216608/

Comment 21

[Matthew N. [:MattN]]

Don't be overwhelmed by the 10 commits as you'll see that none are huge, they are just well split up. I may want to add some tests still but I'll need to think about it more. I may as well assign this to myself since https://reviewboard.mozilla.org/r/216604/diff/#index_header covers what your commit did. We still need a follow-up for sending the selected address to the DOM code.

Comment 22

[Code Review Bot [:reviewbot]]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

https://reviewboard.mozilla.org/r/216596/#review222276


Static analysis found 1 defect in this patch.
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint check path/to/file` (Python/Javascript/wpt)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/payments/res/components/rich-select.js:38
(Diff revision 1)
> +        for (let addedNode of mutation.addedNodes) {
> +          if (addedNode.nodeType != Node.ELEMENT_NODE ||
> +              !addedNode.matches(".rich-option:not(.rich-select-selected-clone)")) {
> +            continue;
> +          }
> +          console.log("moved", addedNode);

Error: Unexpected console statement. [eslint: no-console]

Comment 23

[Matthew N. [:MattN]]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216596/diff/1-2/

Comment 24

[Matthew N. [:MattN]]

Comment on attachment 8946569 [details]
Bug 1427950 - Share common rich-option styles and make rich-select inline-block to improve layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216598/diff/1-2/

Comment 25

[Matthew N. [:MattN]]

Comment on attachment 8946570 [details]
Bug 1427950 - Create address and basic-card option cached children in the constructor and re-use them.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216600/diff/1-2/

Comment 26

[Matthew N. [:MattN]]

Comment on attachment 8946571 [details]
Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216602/diff/1-2/

Comment 27

[Matthew N. [:MattN]]

Comment on attachment 8946572 [details]
Bug 1427950 - Use the <rich-select> 'change' event to update selectedShippingAddress state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216604/diff/1-2/

Comment 28

[Matthew N. [:MattN]]

Comment on attachment 8946573 [details]
Bug 1427950 - Only use the mutation observer for migrating new option children to the popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216606/diff/1-2/

Comment 29

[Matthew N. [:MattN]]

Comment on attachment 8946574 [details]
Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216608/diff/1-2/

Comment 30

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946565 [details]
Bug 1427950 - Add re-render button to PaymentRequest debugging panel.

https://reviewboard.mozilla.org/r/216590/#review222442

Comment 31

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946566 [details]
Bug 1427950 - Have setStateFromParent handle removing deleted GUIDs from the "selected" keys.

https://reviewboard.mozilla.org/r/216592/#review222452

::: toolkit/components/payments/res/debugging.js:143
(Diff revision 1)
>  let buttonActions = {
>    delete1Address() {
>      let savedAddresses = Object.assign({}, requestStore.getState().savedAddresses);
>      delete savedAddresses[Object.keys(savedAddresses)[0]];
> -    requestStore.setState({
> +    // Use setStateFromParent since it ensures there is no dangling
> +    // `selectedShippingAddress` FK reference.

s/FK/foreign key/ since I don't normally see "foreign key" abbreviated like this.

Comment 32

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946567 [details]
Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

https://reviewboard.mozilla.org/r/216594/#review222456

::: toolkit/components/payments/res/containers/address-picker.js:44
(Diff revision 1)
> -    this.dropdown.popupBox.append(...desiredOptions);
> +    for (let option of desiredOptions) {
> +      this.dropdown.popupBox.append(option);
> +    }

Why the change away from ...desiredOptions?

Comment 33

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

https://reviewboard.mozilla.org/r/216596/#review222464

::: toolkit/components/payments/res/components/rich-select.js:188
(Diff revision 2)
>          selectedClone = selectedChild.cloneNode(false);
>          selectedClone.removeAttribute("id");
>          selectedClone.removeAttribute("selected");
> +      } else {
> +        selectedClone = document.createElement("rich-option");
> +        selectedClone.textContent = "(None selected)";

We should be pulling this from a locale file. I don't see a bug on the breakdown spreadsheet for l10n or localization work so we should file that now. I'm OK with pushing this to a follow-up.

Comment 34

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946569 [details]
Bug 1427950 - Share common rich-option styles and make rich-select inline-block to improve layout.

https://reviewboard.mozilla.org/r/216598/#review222466

Comment 35

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946570 [details]
Bug 1427950 - Create address and basic-card option cached children in the constructor and re-use them.

https://reviewboard.mozilla.org/r/216600/#review222468

Comment 36

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946571 [details]
Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

https://reviewboard.mozilla.org/r/216602/#review222470

::: toolkit/components/payments/res/components/rich-select.js:111
(Diff revision 2)
>      }
> +
> +    let option = event.target.closest(".rich-option");
> +    if (this.open && option && !option.matches(".rich-select-selected-clone")) {
> +      this.selectedOption = option;
> +      this._dispatchChangeEvent();

Are you sure you want to dispatch the change event if the user clicks the already selected option?

.rich-select-selected-clone is not set on the popupBox child that is selected, that class is only set on the clone that exists outside of the popupBox.

I understand if you just wanted to block dispatching "change" for if the user clicks on the clone while the popupBox is open, but I think you should also add a check for the [selected] attribute.

Alternatively, you could only go forward with the change if `event.target.closest(".rich-select-popup-box") && !event.target.closest("rich-option").selected`.

::: toolkit/components/payments/res/components/rich-select.js:125
(Diff revision 2)
>        let selectedOption = this.selectedOption;
>        let next = selectedOption.nextElementSibling;
>        if (next) {
>          next.selected = true;
>          selectedOption.selected = false;
> +        this._dispatchChangeEvent();

I don't see anything in this patch set that notifies the page of the change to the selected address, which is something you asked me to add. Is that still coming?

Comment 37

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946572 [details]
Bug 1427950 - Use the <rich-select> 'change' event to update selectedShippingAddress state.

https://reviewboard.mozilla.org/r/216604/#review222474

Comment 38

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946573 [details]
Bug 1427950 - Only use the mutation observer for migrating new option children to the popup.

https://reviewboard.mozilla.org/r/216606/#review222476

Comment 39

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946574 [details]
Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

https://reviewboard.mozilla.org/r/216608/#review222480

::: toolkit/components/payments/test/mochitest/test_rich_select.html:369
(Diff revision 2)
>    await asyncElementRendered();
>  
> -  is(select3.childElementCount, 1, "There should now be one child");
> +  is(select3.childElementCount, 2, "There should still be two children");
>    is(popupBox.childElementCount, 0, "The popup box should not have any children");
>    clonedOption = select3.querySelector(".rich-select-selected-clone");
> -  ok(!clonedOption, "there should be no cloned option");
> +  todo(clonedOption.textContent.includes("None selected"), "the dropdown should show no selection");

Why is this a todo? I believe this should pass now, right?

Comment 40

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8946571 [details]
Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

https://reviewboard.mozilla.org/r/216602/#review222470

> I don't see anything in this patch set that notifies the page of the change to the selected address, which is something you asked me to add. Is that still coming?

I'll work on this in a follow-up bug. You can disregard this question.

Comment 41

[Matthew N. [:MattN]]

Comment on attachment 8946567 [details]
Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

https://reviewboard.mozilla.org/r/216594/#review222456

> Why the change away from ...desiredOptions?

Oops, I meant to change to .appendChild to workaround https://github.com/webcomponents/custom-elements/issues/139 

That may be the reason why I had to add `class="rich-option"` in the test… I can maybe revert that now.

Comment 42

[Matthew N. [:MattN]]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

https://reviewboard.mozilla.org/r/216596/#review222464

> We should be pulling this from a locale file. I don't see a bug on the breakdown spreadsheet for l10n or localization work so we should file that now. I'm OK with pushing this to a follow-up.

Yeah, that's true of other existing strings too. I'll file a bug.

Comment 43

[Matthew N. [:MattN]]

Comment on attachment 8946567 [details]
Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

https://reviewboard.mozilla.org/r/216594/#review222456

> Oops, I meant to change to .appendChild to workaround https://github.com/webcomponents/custom-elements/issues/139 
> 
> That may be the reason why I had to add `class="rich-option"` in the test… I can maybe revert that now.

> That may be the reason why I had to add class="rich-option" in the test… I can maybe revert that now.

Nope, seems like the issue requiring the predefined `class="rich-option"` in the tests still occurs with .appendChild. We don't hit this issue in our shipping code since we don't have option children pre-defined in any selects.

Comment 44

[Matthew N. [:MattN]]

Comment on attachment 8946574 [details]
Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

https://reviewboard.mozilla.org/r/216608/#review222480

> Why is this a todo? I believe this should pass now, right?

It doesn't pass since I basically removed this support as I was re-working things. The main change was that I removed `subtree` from the Mutation Observer since it would also observe all the <span> changes inside the options which was wasteful. I could attach a separate mutation observer on the popupBox to handle removals but since we won't need this for PaymentRequest it didn't seem like it was worth reimplementing now.

Comment 45

[Matthew N. [:MattN]]

Comment on attachment 8946565 [details]
Bug 1427950 - Add re-render button to PaymentRequest debugging panel.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216590/diff/1-2/

Comment 46

[Matthew N. [:MattN]]

Comment on attachment 8946566 [details]
Bug 1427950 - Have setStateFromParent handle removing deleted GUIDs from the "selected" keys.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216592/diff/1-2/

Comment 47

[Matthew N. [:MattN]]

Comment on attachment 8946567 [details]
Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216594/diff/1-2/

Comment 48

[Matthew N. [:MattN]]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216596/diff/2-3/

Comment 49

[Matthew N. [:MattN]]

Comment on attachment 8946569 [details]
Bug 1427950 - Share common rich-option styles and make rich-select inline-block to improve layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216598/diff/2-3/

Comment 50

[Matthew N. [:MattN]]

Comment on attachment 8946570 [details]
Bug 1427950 - Create address and basic-card option cached children in the constructor and re-use them.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216600/diff/2-3/

Comment 51

[Matthew N. [:MattN]]

Comment on attachment 8946571 [details]
Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216602/diff/2-3/

Comment 52

[Matthew N. [:MattN]]

Comment on attachment 8946572 [details]
Bug 1427950 - Use the <rich-select> 'change' event to update selectedShippingAddress state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216604/diff/2-3/

Comment 53

[Matthew N. [:MattN]]

Comment on attachment 8946573 [details]
Bug 1427950 - Only use the mutation observer for migrating new option children to the popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216606/diff/2-3/

Comment 54

[Matthew N. [:MattN]]

Comment on attachment 8946574 [details]
Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216608/diff/2-3/

Comment 55

[Matthew N. [:MattN]]

Comment on attachment 8946565 [details]
Bug 1427950 - Add re-render button to PaymentRequest debugging panel.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216590/diff/2-3/

Comment 56

[Matthew N. [:MattN]]

Comment on attachment 8946566 [details]
Bug 1427950 - Have setStateFromParent handle removing deleted GUIDs from the "selected" keys.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216592/diff/2-3/

Comment 57

[Matthew N. [:MattN]]

Comment on attachment 8946567 [details]
Bug 1427950 - Make <rich-select>'s .selectedOption setter the only supported way to change selection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216594/diff/2-3/

Comment 58

[Matthew N. [:MattN]]

Comment on attachment 8946568 [details]
Bug 1427950 - Change rich-select to default to an indeterminate state when no options are selected.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216596/diff/3-4/

Comment 59

[Matthew N. [:MattN]]

Comment on attachment 8946569 [details]
Bug 1427950 - Share common rich-option styles and make rich-select inline-block to improve layout.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216598/diff/3-4/

Comment 60

[Matthew N. [:MattN]]

Comment on attachment 8946570 [details]
Bug 1427950 - Create address and basic-card option cached children in the constructor and re-use them.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216600/diff/3-4/

Comment 61

[Matthew N. [:MattN]]

Comment on attachment 8946571 [details]
Bug 1427950 - Dispatch a "change" event from <rich-select> when a user changes the selectedness.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216602/diff/3-4/

Comment 62

[Matthew N. [:MattN]]

Comment on attachment 8946572 [details]
Bug 1427950 - Use the <rich-select> 'change' event to update selectedShippingAddress state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216604/diff/3-4/

Comment 63

[Matthew N. [:MattN]]

Comment on attachment 8946573 [details]
Bug 1427950 - Only use the mutation observer for migrating new option children to the popup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216606/diff/3-4/

Comment 64

[Matthew N. [:MattN]]

Comment on attachment 8946574 [details]
Bug 1427950 - Fix test_rich_select.html to handle the indeterminate state with no selection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216608/diff/3-4/

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a159c9b1773
Add re-render button to PaymentRequest debugging panel. r=jaws
https://hg.mozilla.org/integration/autoland/rev/c74872552dba
Have setStateFromParent handle removing deleted GUIDs from the "selected" keys. r=jaws
https://hg.mozilla.org/integration/autoland/rev/71f9f72b2d85
Make <rich-select>'s .selectedOption setter the only supported way to change selection. r=jaws
https://hg.mozilla.org/integration/autoland/rev/025720084eb9
Change rich-select to default to an indeterminate state when no options are selected. r=jaws
https://hg.mozilla.org/integration/autoland/rev/e4e028b18367
Share common rich-option styles and make rich-select inline-block to improve layout. r=jaws
https://hg.mozilla.org/integration/autoland/rev/edf1ecbd9f44
Create address and basic-card option cached children in the constructor and re-use them. r=jaws
https://hg.mozilla.org/integration/autoland/rev/3793ba14d8ae
Dispatch a "change" event from <rich-select> when a user changes the selectedness. r=jaws
https://hg.mozilla.org/integration/autoland/rev/93987f62f19b
Use the <rich-select> 'change' event to update selectedShippingAddress state. r=jaws
https://hg.mozilla.org/integration/autoland/rev/3605aff2cad6
Only use the mutation observer for migrating new option children to the popup. r=jaws
https://hg.mozilla.org/integration/autoland/rev/e64f835d2425
Fix test_rich_select.html to handle the indeterminate state with no selection. r=jaws

Comment 66

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6a159c9b1773
https://hg.mozilla.org/mozilla-central/rev/c74872552dba
https://hg.mozilla.org/mozilla-central/rev/71f9f72b2d85
https://hg.mozilla.org/mozilla-central/rev/025720084eb9
https://hg.mozilla.org/mozilla-central/rev/e4e028b18367
https://hg.mozilla.org/mozilla-central/rev/edf1ecbd9f44
https://hg.mozilla.org/mozilla-central/rev/3793ba14d8ae
https://hg.mozilla.org/mozilla-central/rev/93987f62f19b
https://hg.mozilla.org/mozilla-central/rev/3605aff2cad6
https://hg.mozilla.org/mozilla-central/rev/e64f835d2425