-
Notifications
You must be signed in to change notification settings - Fork 8
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
nested customElements conflict with slots #7
Comments
Thanks for reporting this. We should really add a failing test, then make sure this update works for your specific bug. Will see if I can find some time to do that. |
Unfortunately your fix doesn't seem to solve the problem. I've written a test for tags inside tags without shadow dom and the inner slot seems to be being ignored. Think I'll need more time on this one 🤔 |
This is the failing test if you want to have a go it.only("should allow tags in tags",async()=>{
el = document.createElement('div')
el.innerHTML = `<test-tag><div slot="inner"><test-tag><h2>SUB</h2></test-tag></div></test-tag>`
document.body.appendChild(el)
expect(el.innerHTML).to.equal('<test-tag><h1>Main H1</h1> <div class="content">Main Default <div><test-tag><h1>Main H1</h1> <div class="content"><h2>SUB</h2> <div>Inner Default</div></div></test-tag></div></div></test-tag>')
}) |
sadly i haven't worked with tests yet. NestedCounter.svelte <script>
let count = 0
const increment = () => {
count += 1
}
</script>
<style>
button {
font-family: inherit;
font-size: inherit;
padding: 1em 2em;
color: #ff3e00;
background-color: rgba(255, 62, 0, 0.1);
border-radius: 2em;
border: 2px solid rgba(255, 62, 0, 0);
outline: none;
width: 200px;
font-variant-numeric: tabular-nums;
cursor: pointer;
}
button:focus {
border: 2px solid #ff3e00;
}
button:active {
background-color: rgba(255, 62, 0, 0.2);
}
</style>
<button on:click={increment}>
Clicks: {count}
</button>
<slot name="content" /> Register the customElement import NestedCounter from './components/NestedCounter.svelte'
new Component({
component: NestedCounter ,
tagname:"nested-counter",
}) in the HTML <nested-counter>
<div slot="content">
<nested-counter>
<div slot="content">
nested content
</div>
</nested-counter>
</div>
</nested-counter> |
@Vanillabacke did you manage to solve it? |
@pySilver not yet, i wasn't investing that much time on it. main problem is to keep the state of the children alive in the slots during changing the parent rendering. |
Got some ideas for a fix for this. The reason this appears to be failing is not necessarily because of nested slots per se (although that is and issue for deeper named slots, but that is a separate additional issue!). From memory after tinkering with this yesterday was essentially:
My guess (which I haven't had time to experiment with yet) is we just need to unwind the state back to original on the disconnected callback so that when it connects again in |
Just wanted to confirm here that I think I have this fully resolved, however my code is very dirty at the moment, so it's not ready for a real PR just yet. So, with that disclaimer out of the way, here's a peek at what I'm working on right now in my draft: #18 |
1. Keep track of slots on connectedCallback() and then unwind them symmetrically on disconnectedCallback() to handle state/initialization bugs in case nested elements render from inside out 2. Ensure we restrict selection of slots only to the current element and not nested elements (i.e. from inverse perspective: traverse DOM up from slot and ensure nearest custom element is the current one being initialized).
Good news: I've fixed this issue in version 0.0.8 of my fork of this package: Notes:
|
Update: Mentioning here because originally, even with deferred modules (when all initial HTML was already parsed into the DOM), slots were still an issue. But they were particularly complicated when attempting on page load. This is because the slot content isn't even available yet on |
@patricknelson thanks for your work. hopefully i can test it soon. ❤️ |
Sweet @Vanillabacke. There's a demo now here too: https://svelte-retag.vercel.app/ Migration should be easy, npm i svelte-retag And (depending on what you named it): import svelteTag from 'svelte-tag';
// ... becomes...
import svelteRetag from 'svelte-retag';
// Note: No need for "new" before svelteRetag -- that was never necessary even in svelte-tag.
svelteRetag({
component: HelloWorld,
tagname: 'hello-world',
// etc...
}); Also: I also plan on supporting Svelte 4 whenever it comes out someday and at that point the API to |
Just a note on content layout shift. This can be eliminated with *:not(:defined){
visibility: hidden;
} And setting a known width / height on your custom element. The component will take up the correct amount of space and just become visible once mounted. Obviously you may not always know you components size, but thought it worth mentioning. I seen the above using |
Thanks @crisward that's a great workaround! Should be particularly useful in situations where I might end up mixing both My (totally untested) variant of that would be: *:not(:defined){
visibility: hidden; /* optional UNLESS you have slot content */
min-width: var(--undefined-width, unset); /* required */
min-height: var(--undefined-height, unset); /* required */
} That way I guess you could abstract away some initial/undefined min widths/heights and setup some CSS custom props to initialize those values on a per-element basis. Tiny bit leaky of an abstraction since then you'll need to go yet somewhere else on a per-component (or at least per custom-element) basis and then manually plug in some initial values there. But, it's definitely a very pragmatic workaround. e.g. some-element {
--undefined-width: 400px;
--undefined-height: 200px;
}
another-element {
--undefined-width: 123px;
--undefined-height: 456px;
} Edit: Added examples for clarity. |
At first thanks for that package!
I ran into an issue, where a customElement is nested (for example a nested accordion) and the slot selector is causing an error.
A simple quick fix for it is to change the
querySelectorAll
ingetSlots()
on line95
. my pull request failed, so I will provide my solution for it here as an issue.But there will be still an issue with the state in the nested element. For example, if you expand the nested accordion and collapse the parent and expand it right away, the nested one will reset the state.
cheers! 👍
The text was updated successfully, but these errors were encountered: