Fix the Choices.js quirks by isolating it in a shadow root #8261
Conversation
…ving multiple outlets
|
As discussed in the call, we also want to include the changes from #8154 in this PR. |
There was a problem hiding this comment.
The JS part for this PR works flawlessly ❤️
The CSS part still needs more adjustments, more in the requested changes.
There's still the CSS in tl_filter > tl_select that has to be changed, however that would need the choices-container to get the tl_select class and reset a lot of other stuff, e.g. here:
Before:
After:
Meaning we would most likely need to add the tl_select class within the choices-controller but also another one to reset the styles so we don't have something like this:
IMHO this can simply be fixed by switching to |
Depending on how users were adapting this and adding new fields, this might break their layouts so I am unsure about it. Would adding a class for the new element + adding the styling not make it less prone to errors? Wdyt? |
We can also include all hosts, but I would rather do it the other way round and exclude any descriptions/labels. If targeting |
|
@zoglo You requested changes - is there anything left we need to address? |
The first quirk is the width that has been removed and will change the widths of options accordingly to the selected option within tl_filter.
The other one is somethintg about the shadow-root taking
Maybe the screencast shows it better :) (switching between demo and the current PR) 8261.mp4 |
|
@m-vo as discussed on Slack, I did make sure that the CSS does not change the layouting. 4df465b ensures that the existing widths are preserved for 4ff6c23 fixes a forgotten font-size for choices options d166609 is a bit trickier... I did remove the margin within the shadow DOM but only retain it for a specific case where the instance is not part of the subpanels or filters and where it's not an initialized choices within the shadow dom (Hence the comment). TL;DR - Same styles as before now :) |
|
Thank you @m-vo. |
#8261)" This reverts commit 707a313. # Conflicts: # core-bundle/public/backend.183ec4da.js.LICENSE.txt # core-bundle/public/backend.9ec43b5f.js.LICENSE.txt # core-bundle/public/backend.f83a03fe.js # core-bundle/public/backend.f83a03fe.js.LICENSE.txt # core-bundle/public/entrypoints.json # core-bundle/public/manifest.json
|
I reopened #8248 so we don't forget that this PR has been reverted in 18e970d due to a race condition within the It's a race condition where the |
Fixes #8248
Overview
We had numerous attempts at integrating Choices with our mutation observer based handling of components (#7901, #8075, #8203, …). The main issue is, that Choices wraps/unwraps the underlying select element by removing and readding it to the DOM, which is again triggering mutations. On top there are some global event-listeners. This all screams "race condition".
This PR therefore goes down a completely different route. The strategy basically looks like this:
connect, create a sibling div element (the host) and hide the original element (add.hiddenclass)changeevent on Choices-owned select), update the original element.disconnect/beforeCache, remove the host and show the original element again (remove.hiddenclass)Now, no unwanted mutations occur anymore. In fact, the potentially hundreds of added Choices DOM nodes are also not part of the original DOM which should also make queries etc. on parent elements cheaper.
Implementation details
Choices was a contao-component, now we install it as a npm package (88a718c). We therefore do not load the respective css/js files in
be_mainanymore.Due to isolation, the shadow DOM styles cannot target classes on the parent (custom properties such as our color presets penetrate the boundary, though). There is the
:hostselector, however, that targets the host element. In order to allow styling depending on the current color scheme, I moved the color scheme controller to thehtmlelement and made a small change, so that you now can have multiple outlet targets where the classes are applied (d7f298e). The host elements are now such targets.Because global styles do not penetrate the boundary, we are only applying them to the shadow roots themselves. With webpack that can be done quite nicely: We use an annotated import to get the loader output and then create a
CssStyleSheetobject from it, that is shared across all controller instances (b076c53).Speaking of styles… There were the choices default styles, our adjustments from the contao-component and the base styles (
backend.css) - all of them overwriting each other and sometimes only working with a very specific specificity (say that 3 times in a row). Now that styles are isolated, I moved them in one file (assets/component/choices.pcssimportingchoices.js/public/assets/styles/choices.css) and refactored them. Even with a lot of testing, it is very likely that I broke something while doing so. So please have a close look at this when reviewing.contao--choices:createeventBecause the controller is now basically rewritten, I also added an event that lets you alter the config before the Choices instance is created. If we want to go with this one-line-addition, this PR will also supersede #8115 and #8154.
Usage: