-
Notifications
You must be signed in to change notification settings - Fork 793
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
fix(vdom): hide fallback slot when content present in scoped/non-shadow components #2650
fix(vdom): hide fallback slot when content present in scoped/non-shadow components #2650
Conversation
@adamdbradley @simonhaenisch Let us know if you need/want edits to this. We’re happy to work further if needed. |
Any update on this yet? 😬 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can tell this looks good to go, but I don't have the full picture of the vdom implementation. Giving my approval but it's up to @adamdbradley to double-check and merge this anyway
@simonhaenisch Ok, thanks for the update! |
There is a serious problem here// my-component
import { Component, h } from '@stencil/core';
@Component({
tag: 'my-component',
styleUrl: 'my-component.css',
shadow: false,
})
export class MyComponent {
render() {
return <div><slot><span>my-component fallback</span></slot></div>;
}
}
// my-button
import { Component, h } from '@stencil/core';
@Component({
tag: 'my-button',
styleUrl: 'my-button.css',
shadow: false,
})
export class MyButton {
render() {
return <div>
<my-component>
<div data-txt="Insert into the default slot of my-component">
<slot data-tip="lun-button The default slot of the lun-button"><span>LunButton fallback</span></slot>
</div>
</my-component>
</div>;
}
} <!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0" />
<title>Stencil Component Starter</title>
<script type="module" src="/build/lun-button.esm.js"></script>
<script nomodule src="/build/lun-button.js"></script>
</head>
<body>
<!-- <my-component first="Stencil" last="'Don't call me a framework' JS"></my-component> -->
<my-button></my-button>
</body>
</html> The result of the actual rendering
<my-button class="hydrated">
<!---->
<div>
<my-component class="hydrated">
<!---->
<div>
<slot-fb hidden=""><span>my-component fallback</span></slot-fb>
<div data-txt="Insert into the default slot of my-component">
<!--**lun-button slot-fb should not be hidden**-->
<slot-fb data-tip="lun-button The default slot of the lun-button" hidden=""><span>LunButton fallback</span>
</slot-fb>
</div>
</div>
</my-component>
</div>
</my-button> The actual rendering hides the parent component's slot fallback as well |
A bit of further info, running
The test being:
It's the only test that fails |
This is the same fix as proposed in #2336 by @kevin-coyle-unipro. It was easier to create a fresh PR than somehow coordinate contributing to the other PR. Sorry about that!
This PR also incorporates changes suggested by @simonhaenisch.
For added confidence, I also tested this change against the design system I am working on, and all ~600 tests pass (including visual regression tests).
Fixes #2336. Fixes #2307