Skip to content

Commit

Permalink
fix(runtime): hide slotted content with no destination in scoped comp…
Browse files Browse the repository at this point in the history
…onents (#5135)

* fix(runtime): re-relocate slot if parent element's tagname changes

If a slot is located in an element and that element's tag is dynamically changed (e.g. from `p` to `span`), we need to re-relocate the slot on re-render

STENCIL-672: slot element loses its parent reference and disappears when its parent is rendered conditionally

Fixes: #4284, #3913

* add e2e tests

* code documentation

* put changes behind slot fix flag

* resolve new SNC

* reset `hidden` state of nodes on relocate

It is possible for slotted content to still be invisible in the DOM if the slot was not rendered on the first render. This commit resets the `hidden` attribute of a node on successful relocation.

STENCIL-1053

* hide slot content without a home in `scoped` components

Hides any content that is projected through to a Stencil component that does not have a destination slot. Only for `scoped` components.

Fixes #2877

STENCIL-938

* add e2e tests for hiding content without a slot

* revert karma config

* PR feedback

Co-authored-by: Christian Bromann <git@bromann.dev>

---------

Co-authored-by: Christian Bromann <git@bromann.dev>
  • Loading branch information
tanner-reits and christian-bromann committed Dec 6, 2023
1 parent 735d45a commit 77bce27
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/declarations/stencil-private.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,14 @@ export interface RenderNode extends HostElement {
*/
host?: Element;

/**
* Is initially hidden
* Whether this node was originally rendered with the `hidden` attribute.
*
* Used to reset the `hidden` state of a node during slot relocation.
*/
['s-ih']?: boolean;

/**
* Is Content Reference Node:
* This node is a content reference node.
Expand Down
36 changes: 34 additions & 2 deletions src/runtime/vdom/vdom-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,10 @@ const createElm = (oldParentVNode: d.VNode, newParentVNode: d.VNode, childIndex:
}
}

// This needs to always happen so we can hide nodes that are projected
// to another component but don't end up in a slot
elm['s-hn'] = hostTagName;
if (BUILD.slotRelocation) {
elm['s-hn'] = hostTagName;

if (newVNode.$flags$ & (VNODE_FLAGS.isSlotFallback | VNODE_FLAGS.isSlotReference)) {
// remember the content reference comment
elm['s-sr'] = true;
Expand Down Expand Up @@ -1096,11 +1097,25 @@ render() {
// we're just going to append the node as the last child of the parent. Passing
// `null` as the second arg here will trigger that behavior.
parentNodeRef.insertBefore(nodeToRelocate, insertBeforeNode);

// Reset the `hidden` value back to what it was defined as originally
// This solves a problem where a `slot` is dynamically rendered and `hidden` may have
// been set on content originally, but now it has a slot to go to so it should have
// the value it was defined as having in the DOM, not what we overrode it to.
if (nodeToRelocate.nodeType === NODE_TYPE.ElementNode) {
nodeToRelocate.hidden = nodeToRelocate['s-ih'] ?? false;
}
}
}
} else {
// this node doesn't have a slot home to go to, so let's hide it
if (nodeToRelocate.nodeType === NODE_TYPE.ElementNode) {
// Store the initial value of `hidden` so we can reset it later when
// moving nodes around.
if (isInitialLoad) {
nodeToRelocate['s-ih'] = nodeToRelocate.hidden ?? false;
}

nodeToRelocate.hidden = true;
}
}
Expand All @@ -1118,6 +1133,23 @@ render() {
// always reset
relocateNodes.length = 0;
}

// Hide any elements that were projected through, but don't have a slot to go to.
// Only an issue if there were no "slots" rendered. Otherwise, nodes are hidden correctly.
// This _only_ happens for `scoped` components!
if (BUILD.experimentalSlotFixes && cmpMeta.$flags$ & CMP_FLAGS.scopedCssEncapsulation) {
for (const childNode of rootVnode.$elm$.childNodes) {
if (childNode['s-hn'] !== hostTagName && !childNode['s-sh']) {
// Store the initial value of `hidden` so we can reset it later when
// moving nodes around.
if (isInitialLoad && childNode['s-ih'] == null) {
childNode['s-ih'] = childNode.hidden ?? false;
}

childNode.hidden = true;
}
}
}
};

// slot comment debug nodes only created with the `--debug` flag
Expand Down
30 changes: 30 additions & 0 deletions test/karma/test-app/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,12 @@ export namespace Components {
}
interface SlotFallbackRoot {
}
interface SlotHideContentOpen {
"enabled": boolean;
}
interface SlotHideContentScoped {
"enabled": boolean;
}
interface SlotHtml {
"inc": number;
}
Expand Down Expand Up @@ -1218,6 +1224,18 @@ declare global {
prototype: HTMLSlotFallbackRootElement;
new (): HTMLSlotFallbackRootElement;
};
interface HTMLSlotHideContentOpenElement extends Components.SlotHideContentOpen, HTMLStencilElement {
}
var HTMLSlotHideContentOpenElement: {
prototype: HTMLSlotHideContentOpenElement;
new (): HTMLSlotHideContentOpenElement;
};
interface HTMLSlotHideContentScopedElement extends Components.SlotHideContentScoped, HTMLStencilElement {
}
var HTMLSlotHideContentScopedElement: {
prototype: HTMLSlotHideContentScopedElement;
new (): HTMLSlotHideContentScopedElement;
};
interface HTMLSlotHtmlElement extends Components.SlotHtml, HTMLStencilElement {
}
var HTMLSlotHtmlElement: {
Expand Down Expand Up @@ -1548,6 +1566,8 @@ declare global {
"slot-dynamic-wrapper-root": HTMLSlotDynamicWrapperRootElement;
"slot-fallback": HTMLSlotFallbackElement;
"slot-fallback-root": HTMLSlotFallbackRootElement;
"slot-hide-content-open": HTMLSlotHideContentOpenElement;
"slot-hide-content-scoped": HTMLSlotHideContentScopedElement;
"slot-html": HTMLSlotHtmlElement;
"slot-light-dom-content": HTMLSlotLightDomContentElement;
"slot-light-dom-root": HTMLSlotLightDomRootElement;
Expand Down Expand Up @@ -1896,6 +1916,12 @@ declare namespace LocalJSX {
}
interface SlotFallbackRoot {
}
interface SlotHideContentOpen {
"enabled"?: boolean;
}
interface SlotHideContentScoped {
"enabled"?: boolean;
}
interface SlotHtml {
"inc"?: number;
}
Expand Down Expand Up @@ -2095,6 +2121,8 @@ declare namespace LocalJSX {
"slot-dynamic-wrapper-root": SlotDynamicWrapperRoot;
"slot-fallback": SlotFallback;
"slot-fallback-root": SlotFallbackRoot;
"slot-hide-content-open": SlotHideContentOpen;
"slot-hide-content-scoped": SlotHideContentScoped;
"slot-html": SlotHtml;
"slot-light-dom-content": SlotLightDomContent;
"slot-light-dom-root": SlotLightDomRoot;
Expand Down Expand Up @@ -2255,6 +2283,8 @@ declare module "@stencil/core" {
"slot-dynamic-wrapper-root": LocalJSX.SlotDynamicWrapperRoot & JSXBase.HTMLAttributes<HTMLSlotDynamicWrapperRootElement>;
"slot-fallback": LocalJSX.SlotFallback & JSXBase.HTMLAttributes<HTMLSlotFallbackElement>;
"slot-fallback-root": LocalJSX.SlotFallbackRoot & JSXBase.HTMLAttributes<HTMLSlotFallbackRootElement>;
"slot-hide-content-open": LocalJSX.SlotHideContentOpen & JSXBase.HTMLAttributes<HTMLSlotHideContentOpenElement>;
"slot-hide-content-scoped": LocalJSX.SlotHideContentScoped & JSXBase.HTMLAttributes<HTMLSlotHideContentScopedElement>;
"slot-html": LocalJSX.SlotHtml & JSXBase.HTMLAttributes<HTMLSlotHtmlElement>;
"slot-light-dom-content": LocalJSX.SlotLightDomContent & JSXBase.HTMLAttributes<HTMLSlotLightDomContentElement>;
"slot-light-dom-root": LocalJSX.SlotLightDomRoot & JSXBase.HTMLAttributes<HTMLSlotLightDomRootElement>;
Expand Down
25 changes: 25 additions & 0 deletions test/karma/test-app/slot-hide-content/cmp-open.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Component, h, Host, Prop } from '@stencil/core';

@Component({
tag: 'slot-hide-content-open',
scoped: false,
shadow: false,
})
export class SlotHideContentOpen {
@Prop() enabled = false;

render() {
return (
<Host>
<p>Test</p>
{this.enabled && (
<div class="slot-wrapper">
<slot>
<span>fallback default slot</span>
</slot>
</div>
)}
</Host>
);
}
}
24 changes: 24 additions & 0 deletions test/karma/test-app/slot-hide-content/cmp-scoped.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Component, h, Host, Prop } from '@stencil/core';

@Component({
tag: 'slot-hide-content-scoped',
scoped: true,
})
export class SlotHideContentScoped {
@Prop() enabled = false;

render() {
return (
<Host>
<p>Test</p>
{this.enabled && (
<div class="slot-wrapper">
<slot>
<span>fallback default slot</span>
</slot>
</div>
)}
</Host>
);
}
}
20 changes: 20 additions & 0 deletions test/karma/test-app/slot-hide-content/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<meta charset="utf8" />
<script src="./build/testapp.esm.js" type="module"></script>
<script src="./build/testapp.js" nomodule></script>

<slot-hide-content-scoped class="test-cmp">
<p id="slotted-1">Hello</p>
</slot-hide-content-scoped>

<slot-hide-content-open class="test-cmp">
<p id="slotted-2">Hello</p>
</slot-hide-content-open>

<button type="button">Enable slot</button>

<script>
document.querySelector('button').addEventListener('click', () => {
document.querySelectorAll('.test-cmp').forEach((ref) => ref.setAttribute('enabled', true));
});
</script>
51 changes: 51 additions & 0 deletions test/karma/test-app/slot-hide-content/karma.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { setupDomTests, waitForChanges } from '../util';

/**
* Tests for projected content to be hidden in a `scoped` component
* when it has no destination slot.
*/
describe('slot-hide-content', function () {
const { setupDom, tearDownDom } = setupDomTests(document);
let app: HTMLElement | undefined;
let host: HTMLElement | undefined;

beforeEach(async () => {
app = await setupDom('/slot-hide-content/index.html');
});

afterEach(tearDownDom);

describe('scoped encapsulation', () => {
it('should hide content when no slot is provided', async () => {
host = app.querySelector('slot-hide-content-scoped');
const slottedContent = host.querySelector('#slotted-1');

expect(slottedContent).toBeDefined();
expect(slottedContent.hasAttribute('hidden')).toBe(true);
expect(slottedContent.parentElement.tagName).toContain('SLOT-HIDE-CONTENT-SCOPED');

document.querySelector('button').click();
await waitForChanges();

expect(slottedContent.hasAttribute('hidden')).toBe(false);
expect(slottedContent.parentElement.classList).toContain('slot-wrapper');
});
});

describe('no encapsulation', () => {
it('should not hide content when no slot is provided', async () => {
host = app.querySelector('slot-hide-content-open');
const slottedContent = host.querySelector('#slotted-2');

expect(slottedContent).toBeDefined();
expect(slottedContent.hasAttribute('hidden')).toBe(false);
expect(slottedContent.parentElement.tagName).toContain('SLOT-HIDE-CONTENT-OPEN');

document.querySelector('button').click();
await waitForChanges();

expect(slottedContent.hasAttribute('hidden')).toBe(false);
expect(slottedContent.parentElement.classList).toContain('slot-wrapper');
});
});
});

0 comments on commit 77bce27

Please sign in to comment.