Picker fails to update when adding a new item and setting as the current value

[bengfarrell]

Expected Behaviour

During a single render update, I should be able to add a new item to my data model driving the menu-items, as well as selecting this new item. The label should update to the current value and reflect that it exists in the list by using non-placeholder (non-italic) text

Actual Behaviour

The picker label does not update when adding a new item and selecting it

Reproduce Scenario (including but not limited to)

https://webcomponents.dev/edit/iiBMUXs1JiTFL6Hf4y9A/src/index.ts

Steps to Reproduce

Create a button to add menu-items and make the selection. Click the button and watch the label not udpate

Platform and Version

Picker 0.3.1 on Chrome

[Westbrook]

This is the fix that I'm theorizing right now: https://webcomponents.dev/edit/G5DPiQudnGayyu9qO9ze/src/index.ts. I'll submit a PR for this, as I see it as a qualitatively better solution that what's currently available.

I do think it's a bit short term, however. The ability to place items directly into the <sp-picker> that @adixon-adobe added recently implies a handful of things that we may need to take into account for this element to be "locked down".

While manageSelection here isn't called often, querying children every time it's called seems a bit busy. It would be better if we could in some way bind the updateMenuItems call to the slot change or similar, but with us manually taking items in and out of the DOM that could get a bit tricky.

While this solution works when !this.open, it does not while this.open. Without the sp-menu element to cache a reference to, I'm not sure that an implementor can easily add options while the menu is open. This would likely be a slot change or Mutation Observer situation that would be bound to the same difficulties as listed above.

It's also possible that I'm over thinking this. For instance when you visit https://webcomponents.dev/edit/biTK3akZblgHf3d0RDYX/src/index.js across various browsers in macOS; Chrome will actually close/open the menu for you when you add the new options, Firefox does exactly what you'd expect, and Safari does not allow the new option until you've manually closed and opened the menu. So, we're not in the worst of company struggling a little bit as far as how to address this.

In the sp-combobox research, we're thinking about a API that accepts both declarative and imperative children, that might be able to address some of these things, but might further complicate things. I'll report back when I have a little firmer idea of what we'd like to propose there.

[tirithen]

I have also been trying to workaround these problems the last days (using @spectrum-web-components/picker 0.4.4) in a project where we need to replace the options in a sp-picker depending on which options the user selects elsewhere in the user interface.

We tried adding additional workaround code to manually close the picker if it was open, and then wait until all the sp-menu-item elements was moved back before re-rendering our own component, but then we got into problems with setting the values.

Adding additional code to set selectedItem property helped a bit, but there seem to be a lot of edge cases to prove 100% that all attributes/properties are in sync with the child DOM tree of the component.

It would be super helpful if possible to ensure that both sp-picker and all of its child sp-menu-item elements always have their properties in sync. Maybe a MutationObserver or similar is needed to ensure that. I can see that it is tricky as the API surface is large and in this case even spans over more than one component.

Thanks for an otherwise visually nice looking set of components. Fingers crossed for a good solution for this soon.

[Westbrook]

@tirithen would you be able to share a repro on the situation you are seeing here? I’d love to ensure that it’s supported by our API, but I’m not fully clear on your workflow here. Thanks in advance.

[tirithen]

Thanks for the reply, I would appreciate that but I need some time to break it out to a minimal setup as sp-picker is part of another lit-element component that in turn is used in a Vue app. UX vice it is about allowibg the end user to restrict the options for a sp-picker by toggling sp-switch elements to exclude options on the sp-picker. Therefore we need to re-render the sp-menu-item elements inside the sp-picker, and also ideally keeping the state/value of the sp-picker (as long as the selected value was not removed). I'll try to create a repo with a more minimal setup during the coming days to help pinpoint the problems. 3 maj 2021 18:57:22 Westbrook Johnson ***@***.***>:

…

@tirithen[https://github.com/tirithen] would you be able to share a repro on the situation you are seeing here? I’d love to ensure that it’s supported by our API, but I’m not fully clear on your workflow here. Thanks in advance. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub[#1331 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAB4AM6JQGTKED6OTKDIX3TTL3IXDANCNFSM42AWS4WA]. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEgAAABICAYAAABV7bNHAAAAAXNSR0IArs4c6QAAAARzQklUCAgICHwIZIgAAAArSURBVHic7cEBDQAAAMKg909tDjegAAAAAAAAAAAAAAAAAAAAAAAAAAA+DFFIAAEctgHwAAAAAElFTkSuQmCC###24x24:true###][Spårningsbild][https://github.com/notifications/beacon/AAB4AM4FPPJM56LPHMZ4TALTL3IXDA5CNFSM42AWS4WKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOGGHA5YQ.gif]

[Westbrook]

@tirithen, maybe this could help? https://webcomponents.dev/edit/xLImpVq4PYA0HUQdls6q/src/index.ts Here, the first picker chooses how many options will display in the second picker. The update button toggles between the two lists of options for the second picker and ensures the first picker stays up to date. Super contrived, but maybe it sheds some light on what you're looking to do here?

[tirithen]

@Westbrook sorry for late reply, the example you setup made the issues I have easy to reproduce, thanks, also a really nice page to experiment and share web components. :)

I made the updateOptions run on a timer to simulate the problems we have been having in the project I work on where we change the options externally depending on the users input elsewhere (which is also async).

A GIF showing the extra options and the crash when it fails to replace an element, probably as it is currently moved elsewhere as the dropdown is open.

The component keeps copies of the old options and eventually crashes, it seem to happen especially when the dropdown is open. Maybe if you would clone the elements instead of detaching/attaching them to the DOM it might be easier to prevent those kind of crashes/duplications. It it tricky as the API surface of components are large and all properties/attributes and in this case children need to be in sync at all times.

The modified version: https://webcomponents.dev/edit/phC1LLFUDc4QdHapBAqr/src/index.ts

To reproduce:

• Open the dropdown when one of the options from options1 is shown
• Wait for a few seconds, now you should that there is always 4 options and the last one is always from options1 even of the first three are from options2
• Click outside to close the dropdown, then the component crashes

It is a nice looking component so looking forward to using it once it is working, meanwhile I resorted to a plain <select> tag styled a bit similar to the Spectrum look and feel.

There was also problems with assigning the selected value in the project I worked on, but I'm afraid that I do not currently remember how to reproduce, anyhow this is probably a good starting point, maybe that will also work better once these bugs have been taken care of.

And again, thanks for an otherwise nice looking and easy to use component library.

[Westbrook]

Thanks for the reproduction case. I definitely would not expect this to work right now based on the approach we take to managing these elements. Your best hope at a work around today would be to know you are receiving new options, close the menu, apply the new options, and reopen the menu. It's possible that future work could support this, but I'm not sure when we'd be able to prioritize addressing this. In theory the concepts underlying this change are being researched right now in our Combobox patter development, but how soon that completes and whether or not we choose to bring the ideas applied there in back to the Picker UI is as yet unclear.

Under a short review of the browser ecosystem for this, it also seems that the native select element also supports this in a very bumpy and somewhat unpredictable way: https://codepen.io/Westbrook/pen/wvJzQMq?editors=0010

In the link above with macOS you'll see Chrome visibly close and the reopen the select menu for every option addition that is made while the menu is open.

In Safari, the menu contents will NOT change while open. Closing and opening again will display an update (at yet once again static) list of options.

Firefox seems to work as expected adding the option elements with no visible disruption. This seems also to be true in Edge for macOS, as well as all three browsers in Window.

[tirithen]

No problem :)

I guess that it does not matter much whether the dropdown closes or not when the options are updated, the problem is when it crashes, or the old child option tags stays and become out of sync with the options property as that makes the component unreliable.

We did some tests to workaround by trying to detect if the dropdown was open or not, but it simply got to complex. As the open property acts as a setter to change the state .open === false before all the child nodes has been moved back, and looking into the .shadowRoot to see if the children has been moved back yet got to messy to solve from outside the component.

I guess that the .options property right now is has setter with side effects that are a bit to tricky to adjust for. Hopefully that complexity can be solved internally by the component with some insights from the Combobox that you mentioned.

As the public API consists of several properties (.options, .open, .selectedItem) and child sp-menu-item elements and they have interdependence on each other I can see that this is tricky to get right, especially if the children also changes parent when .open is changed.

Just as a suggestion, if the children was copied instead of moved to the end of the tag, maybe the .open property would not need to be as tightly coupled with the others as the options could then update independent of the open state.

[Westbrook]

Will be addressed by #2086