symfony
[Autocomplete] Fix grouped options order
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| Issues | Fix #1616 |
| License | MIT |
The bug is being caused by TomSelect not preserving the order of the option elements in the select as we select the options in the dropdown. In this case, the MutationObserver callback uses the optgroup element as a parameter to "store" the group (if any) to which the option belongs. However, once the option is selected, it no longer has an optgroup as its parentElement (a problem caused by the aforementioned bug).
As I see it, there is no need to "store" the option's group since, in any case, all
A callback was added for options added through addOption, but the caveat is that it needs the parameter user_created=true to trigger the 'option_add' event.
Oh i'm so sorry @IndraGunawan .. i remember seeing this PR.... and i forgot to come back on it !
For Autocomplete it seems a very smart & efficient fix: thank you very much for this!
Just, before we merge i'd like to be sure this does not add problems/side effets when used with LiveComponent (to be transparent: i'm 90% certain this will in fact solve/improve things..... but i'd like to be 100%)
So... did you try within a LiveComponent ? I'm not talking about coding a test or adding anything more in this PR.. But at least one of us should test manually to check this new behaviour has no impact.
I can help if you want / don't have time, feel free to ask!
A good example would be to add groups to the dependant form fields demo of the ux.website (present in the same repository) and see if everything is still working.
⚠️ @kbond
I marked this as reviewed/accepted because the PR in itself is ok !
Let's just wait we check everything is ok with LiveComponent before merging.
@smnandre Really appreciate your feedback because I myself didn't know if it was a good solution. I was waiting for feedback from you. Regarding LiveComponent, to be honest, I haven't tested with it yet, but I will later and let you know.
@smnandre Sorry for the delay. As you suggested, I grouped the options from the dependent form on the UX website and everything seems fine.
Would you like me to update this PR with the changes to the dependent form?
@zavarock thank you for taking some time to check this!
No need to put it in the PR, and i'd rather keep this particular demo the simplest possible (as it's one of the most seen...)
But after this PR, it could be a good idea to add a couple of demo / code samples for Autcomplete (only if you want and have time of course)
@smnandre Sure, no problems! I just have two questions if you don't mind:
- You're talking about adding a new demo on this page, right?
- Is it better to create a new PR to avoid mixing things? Or can I just update this one?
-
More probably a dedicated page. I will post a page during the week to explain how to do this properly (short answer, same thing as the LiveComponent demos... but i'll try to isolate more the demos)
-
I personnaly find better to separate code PRs and website ones.. but that's 100% open to debate :)
How about the status to integrate that issue in the next release, as it is important for the application to have a proper grouped dropdown. As i read it the issue is fixed and working, the demo could be a separate thing to update.
📊 Packages dist files size difference
Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR. Please review the changes and make sure they are expected.
| File | Before (Size / Gzip) | After (Size / Gzip) |
|---|---|---|
| Autocomplete | ||
controller.js |
15.04 kB
/ 3.62 kB
|
16.47 kB+9% 📈
/ 3.93 kB+9% 📈
|
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@smnandre i'm so sorry for following this up over and over again but can we wrap up this PR in the near future?