Sl-dropdown is broken when removed and appended back to DOM

[ex37]

Describe the bug

In dropdown component, this.popover is initialized in firstUpdated hook, but is destroyed in disconnectedCallback , thus when removing the dropdown from DOM and then appending again the popover is broken.

To Reproduce

1. Create a sl-dropdown
2. Programmatically remove it from DOM
3. Append back to DOM
4. Open and then close the dropwdown, it's remaining open, cause this.popover.hide() can't be called (this.popover is destroyed)

Expected behavior
Dropdown should keep working as intended when programmatically moving within DOM.

[claviska]

Thanks for reporting this. I was able to replicate the issue and fixed it in d7bf0bd.

[ex37]

Hi @claviska thanks for fast response!
I spent some time and reviewed other occurrences where firstUpdated is used, and just by looking at the code (haven't tried to reproduce any of these), here is what I found:

https://github.com/shoelace-style/shoelace/blob/next/src/components/icon-button/icon-button.ts#L41

focusVisible.observe(this.button); should be moved to connectedCallback

https://github.com/shoelace-style/shoelace/blob/next/src/components/rating/rating.ts#L66
https://github.com/shoelace-style/shoelace/blob/next/src/components/details/details.ts#L68

exactly same issue

https://github.com/shoelace-style/shoelace/blob/next/src/components/tooltip/tooltip.ts#L103

this.popover = new Popover(this.target, this.positioner); should be moved to connectedCallback,
as well as all of the event listeners. Maybe also this.target = this.getTarget();, not sure about this one, but overall in the current state I find tooltip component not enought dynamic and ended up wraping it into my own one. If you are interested I can collaborate my thoughts on this component.

https://github.com/shoelace-style/shoelace/blob/next/src/components/textarea/textarea.ts#L141
this.resizeObserver.observe(this.input); should be moved

https://github.com/shoelace-style/shoelace/blob/next/src/components/tab-group/tab-group.ts#L65
this.resizeObserver.observe(this.nav); and the creation of this.mutationObserver should be moved

Hope this helps!