Skip to content

Commit 879779b

Browse files
driskullbenelan
authored andcommitted
fix(modal): fix rendering tied to named-slot content (#10469)
**Related Issue:** #6059 ## Summary - remove use of `getSlotted` utility - replace with `slotchange` event and `@State` variables to update the display of elements. - existing tests should suffice
1 parent a45d137 commit 879779b

File tree

2 files changed

+55
-39
lines changed

2 files changed

+55
-39
lines changed

packages/calcite-components/src/components/modal/modal.e2e.ts

+5-6
Original file line numberDiff line numberDiff line change
@@ -577,17 +577,16 @@ describe("calcite-modal", () => {
577577

578578
it("renders correctly with no footer", async () => {
579579
const page = await newE2EPage();
580-
await page.setContent(`
581-
<calcite-modal>
580+
await page.setContent(html`
581+
<calcite-modal open>
582582
<calcite-button slot="primary">TEST</calcite-button>
583583
</calcite-modal>
584584
`);
585-
let footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer"));
586-
expect(footer).toBeDefined();
585+
const footer = await page.find("calcite-modal >>> .footer");
586+
expect(await footer.isVisible()).toBe(true);
587587
await page.$eval("calcite-button", (el) => el.parentElement.removeChild(el));
588588
await page.waitForChanges();
589-
footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer"));
590-
expect(footer).toBeFalsy();
589+
expect(await footer.isVisible()).toBe(false);
591590
});
592591

593592
it("should render calcite-scrim with default background color", async () => {

packages/calcite-components/src/components/modal/modal.tsx

+50-33
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,10 @@ import {
1212
VNode,
1313
Watch,
1414
} from "@stencil/core";
15-
import {
16-
ConditionalSlotComponent,
17-
connectConditionalSlotComponent,
18-
disconnectConditionalSlotComponent,
19-
} from "../../utils/conditionalSlot";
2015
import {
2116
ensureId,
2217
focusFirstTabbable,
23-
getSlotted,
18+
slotChangeGetAssignedElements,
2419
slotChangeHasAssignedElement,
2520
} from "../../utils/dom";
2621
import {
@@ -80,7 +75,6 @@ logger.deprecated("component", {
8075
})
8176
export class Modal
8277
implements
83-
ConditionalSlotComponent,
8478
OpenCloseComponent,
8579
FocusTrapComponent,
8680
LoadableComponent,
@@ -194,8 +188,6 @@ export class Modal
194188
this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
195189
this.cssVarObserver?.observe(this.el, { attributeFilter: ["style"] });
196190
this.updateSizeCssVars();
197-
this.updateFooterVisibility();
198-
connectConditionalSlotComponent(this);
199191
connectLocalized(this);
200192
connectMessages(this);
201193
connectFocusTrap(this);
@@ -205,7 +197,6 @@ export class Modal
205197
this.removeOverflowHiddenClass();
206198
this.mutationObserver?.disconnect();
207199
this.cssVarObserver?.disconnect();
208-
disconnectConditionalSlotComponent(this);
209200
deactivateFocusTrap(this);
210201
disconnectLocalized(this);
211202
disconnectMessages(this);
@@ -238,7 +229,7 @@ export class Modal
238229
<div class={CSS.header}>
239230
{this.renderCloseButton()}
240231
<header class={CSS.title}>
241-
<slot name={CSS.header} />
232+
<slot name={CSS.header} onSlotchange={this.handleHeaderSlotChange} />
242233
</header>
243234
</div>
244235
{this.renderContentTop()}
@@ -249,7 +240,7 @@ export class Modal
249240
}}
250241
ref={(el) => (this.modalContent = el)}
251242
>
252-
<slot name={SLOTS.content} />
243+
<slot name={SLOTS.content} onSlotchange={this.handleContentSlotChange} />
253244
</div>
254245
{this.renderContentBottom()}
255246
{this.renderFooter()}
@@ -260,19 +251,19 @@ export class Modal
260251
}
261252

262253
renderFooter(): VNode {
263-
return this.hasFooter ? (
264-
<div class={CSS.footer} key="footer">
254+
return (
255+
<div class={CSS.footer} hidden={!this.hasFooter} key="footer">
265256
<span class={CSS.back}>
266-
<slot name={SLOTS.back} />
257+
<slot name={SLOTS.back} onSlotchange={this.handleBackSlotChange} />
267258
</span>
268259
<span class={CSS.secondary}>
269-
<slot name={SLOTS.secondary} />
260+
<slot name={SLOTS.secondary} onSlotchange={this.handleSecondarySlotChange} />
270261
</span>
271262
<span class={CSS.primary}>
272-
<slot name={SLOTS.primary} />
263+
<slot name={SLOTS.primary} onSlotchange={this.handlePrimarySlotChange} />
273264
</span>
274265
</div>
275-
) : null;
266+
);
276267
}
277268

278269
renderContentTop(): VNode {
@@ -357,7 +348,7 @@ export class Modal
357348
modalContent: HTMLDivElement;
358349

359350
private mutationObserver: MutationObserver = createObserver("mutation", () =>
360-
this.handleMutationObserver(),
351+
this.updateFocusTrapElements(),
361352
);
362353

363354
private cssVarObserver: MutationObserver = createObserver("mutation", () => {
@@ -380,7 +371,24 @@ export class Modal
380371

381372
@State() cssHeight: string | number;
382373

383-
@State() hasFooter = true;
374+
@State() hasFooter = false;
375+
376+
@Watch("hasBack")
377+
@Watch("hasPrimary")
378+
@Watch("hasSecondary")
379+
handleHasFooterChange(): void {
380+
this.hasFooter = this.hasBack || this.hasPrimary || this.hasSecondary;
381+
}
382+
383+
@State() titleEl: HTMLElement;
384+
385+
@State() contentEl: HTMLElement;
386+
387+
@State() hasBack = false;
388+
389+
@State() hasPrimary = false;
390+
391+
@State() hasSecondary = false;
384392

385393
@State() hasContentTop = false;
386394

@@ -474,6 +482,26 @@ export class Modal
474482
//
475483
//--------------------------------------------------------------------------
476484

485+
private handleHeaderSlotChange = (event: Event): void => {
486+
this.titleEl = slotChangeGetAssignedElements<HTMLElement>(event)[0];
487+
};
488+
489+
private handleContentSlotChange = (event: Event): void => {
490+
this.contentEl = slotChangeGetAssignedElements<HTMLElement>(event)[0];
491+
};
492+
493+
private handleBackSlotChange = (event: Event): void => {
494+
this.hasBack = slotChangeHasAssignedElement(event);
495+
};
496+
497+
private handlePrimarySlotChange = (event: Event): void => {
498+
this.hasPrimary = slotChangeHasAssignedElement(event);
499+
};
500+
501+
private handleSecondarySlotChange = (event: Event): void => {
502+
this.hasSecondary = slotChangeHasAssignedElement(event);
503+
};
504+
477505
private setTransitionEl = (el: HTMLDivElement): void => {
478506
this.transitionEl = el;
479507
};
@@ -533,11 +561,9 @@ export class Modal
533561
await componentOnReady(this.el);
534562
this.el.addEventListener("calciteModalOpen", this.openEnd);
535563
this.opened = true;
536-
const titleEl = getSlotted(this.el, SLOTS.header);
537-
const contentEl = getSlotted(this.el, SLOTS.content);
538564

539-
this.titleId = ensureId(titleEl);
540-
this.contentId = ensureId(contentEl);
565+
this.titleId = ensureId(this.titleEl);
566+
this.contentId = ensureId(this.contentEl);
541567

542568
if (!this.embedded) {
543569
if (totalOpenModals === 0) {
@@ -582,15 +608,6 @@ export class Modal
582608
document.documentElement.style.setProperty("overflow", initialDocumentOverflowStyle);
583609
}
584610

585-
private handleMutationObserver = (): void => {
586-
this.updateFooterVisibility();
587-
this.updateFocusTrapElements();
588-
};
589-
590-
private updateFooterVisibility = (): void => {
591-
this.hasFooter = !!getSlotted(this.el, [SLOTS.back, SLOTS.primary, SLOTS.secondary]);
592-
};
593-
594611
private updateSizeCssVars = (): void => {
595612
this.cssWidth = getComputedStyle(this.el).getPropertyValue("--calcite-modal-width");
596613
this.cssHeight = getComputedStyle(this.el).getPropertyValue("--calcite-modal-height");

0 commit comments

Comments
 (0)