Skip to content

[LiveComponent][Autocomplete] Adding support for libraries to play nicely #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 14, 2022

Conversation

weaverryan
Copy link
Member

Q A
Bug fix? yes
New feature? no (but yes to BC break)
Tickets Fixes #407 and helps with #354
License MIT

See the commit for more information - this was a VERY pesky bug.

Cheers!

@weaverryan weaverryan changed the title [LoveComponent][Autocomplete] Adding support for libraries to play nicely [LiveComponent][Autocomplete] Adding support for libraries to play nicely Aug 10, 2022
@weaverryan weaverryan force-pushed the autocomplete-destroy-fix branch from a4e8461 to c8f51be Compare August 10, 2022 20:36
@akyoscommunication
Copy link

Tried it and that's perfect! 👏 👏 👏

…cely together

This includes several important points:

A) In autocomplete, data-live-ignore was added so that, when a "live component" re-renders,
the element is not affected.

B) For morphdom, the node key was changed from "id" to "data-live-id". This was due to
a very complex "bad situation" that arised when autocomplete+live components. TomSelect
finds the label for the element and give it an id attribute, which helps with the
aria- attributes. However, this caused, on re-render, morphdom to think the "new label"
did not match the old label. This small difference caused the elements in the parent
div to be re-added one-by-one. Most importantly, a very innocent fromEl.appendChild(selectEl)
is called deep in morphdom. In reality, selectEl already exists in fromEl... and the purpose
of this call is actually just to help order things in the correct way. However, appendChild()
caused the select element to momentarily be removed from the DOM then re-added. Stimulus
noticed this and disconnected and then reconnected the autocomplete controller, causing
the element to re-initialize (this, at the very least, would cause the element to lose focus).

Anyways, this whole bad situation (which, is a bit poorly handled by morphdom), was caused
by morphdom being confused by the "id" attribute on the label, which is present in the DOM
but not in the re-rendered HTML. To avoid side effects like this, the nodeKey was changed
to data-live-id.

C) In live components, we also fixed a bug where if an element had data-live-ignore, it could
be, incorrectly, removed on re-render if not present.
@weaverryan weaverryan force-pushed the autocomplete-destroy-fix branch from dbb9c03 to c4791fc Compare August 14, 2022 12:15
@weaverryan weaverryan merged commit 5dcd76a into symfony:2.x Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Autocomplete] and [LiveComponent] don't play well together
2 participants