Skip to content

Commit

Permalink
fix(modal): fix rendering tied to named-slot content (#10469)
Browse files Browse the repository at this point in the history
**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
  • Loading branch information
driskull authored and benelan committed Feb 8, 2025
1 parent a45d137 commit 879779b
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 39 deletions.
11 changes: 5 additions & 6 deletions packages/calcite-components/src/components/modal/modal.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -577,17 +577,16 @@ describe("calcite-modal", () => {

it("renders correctly with no footer", async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-modal>
await page.setContent(html`
<calcite-modal open>
<calcite-button slot="primary">TEST</calcite-button>
</calcite-modal>
`);
let footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer"));
expect(footer).toBeDefined();
const footer = await page.find("calcite-modal >>> .footer");
expect(await footer.isVisible()).toBe(true);
await page.$eval("calcite-button", (el) => el.parentElement.removeChild(el));
await page.waitForChanges();
footer = await page.$eval("calcite-modal", (el) => el.shadowRoot.querySelector(".footer"));
expect(footer).toBeFalsy();
expect(await footer.isVisible()).toBe(false);
});

it("should render calcite-scrim with default background color", async () => {
Expand Down
83 changes: 50 additions & 33 deletions packages/calcite-components/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,10 @@ import {
VNode,
Watch,
} from "@stencil/core";
import {
ConditionalSlotComponent,
connectConditionalSlotComponent,
disconnectConditionalSlotComponent,
} from "../../utils/conditionalSlot";
import {
ensureId,
focusFirstTabbable,
getSlotted,
slotChangeGetAssignedElements,
slotChangeHasAssignedElement,
} from "../../utils/dom";
import {
Expand Down Expand Up @@ -80,7 +75,6 @@ logger.deprecated("component", {
})
export class Modal
implements
ConditionalSlotComponent,
OpenCloseComponent,
FocusTrapComponent,
LoadableComponent,
Expand Down Expand Up @@ -194,8 +188,6 @@ export class Modal
this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
this.cssVarObserver?.observe(this.el, { attributeFilter: ["style"] });
this.updateSizeCssVars();
this.updateFooterVisibility();
connectConditionalSlotComponent(this);
connectLocalized(this);
connectMessages(this);
connectFocusTrap(this);
Expand All @@ -205,7 +197,6 @@ export class Modal
this.removeOverflowHiddenClass();
this.mutationObserver?.disconnect();
this.cssVarObserver?.disconnect();
disconnectConditionalSlotComponent(this);
deactivateFocusTrap(this);
disconnectLocalized(this);
disconnectMessages(this);
Expand Down Expand Up @@ -238,7 +229,7 @@ export class Modal
<div class={CSS.header}>
{this.renderCloseButton()}
<header class={CSS.title}>
<slot name={CSS.header} />
<slot name={CSS.header} onSlotchange={this.handleHeaderSlotChange} />
</header>
</div>
{this.renderContentTop()}
Expand All @@ -249,7 +240,7 @@ export class Modal
}}
ref={(el) => (this.modalContent = el)}
>
<slot name={SLOTS.content} />
<slot name={SLOTS.content} onSlotchange={this.handleContentSlotChange} />
</div>
{this.renderContentBottom()}
{this.renderFooter()}
Expand All @@ -260,19 +251,19 @@ export class Modal
}

renderFooter(): VNode {
return this.hasFooter ? (
<div class={CSS.footer} key="footer">
return (
<div class={CSS.footer} hidden={!this.hasFooter} key="footer">
<span class={CSS.back}>
<slot name={SLOTS.back} />
<slot name={SLOTS.back} onSlotchange={this.handleBackSlotChange} />
</span>
<span class={CSS.secondary}>
<slot name={SLOTS.secondary} />
<slot name={SLOTS.secondary} onSlotchange={this.handleSecondarySlotChange} />
</span>
<span class={CSS.primary}>
<slot name={SLOTS.primary} />
<slot name={SLOTS.primary} onSlotchange={this.handlePrimarySlotChange} />
</span>
</div>
) : null;
);
}

renderContentTop(): VNode {
Expand Down Expand Up @@ -357,7 +348,7 @@ export class Modal
modalContent: HTMLDivElement;

private mutationObserver: MutationObserver = createObserver("mutation", () =>
this.handleMutationObserver(),
this.updateFocusTrapElements(),
);

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

@State() cssHeight: string | number;

@State() hasFooter = true;
@State() hasFooter = false;

@Watch("hasBack")
@Watch("hasPrimary")
@Watch("hasSecondary")
handleHasFooterChange(): void {
this.hasFooter = this.hasBack || this.hasPrimary || this.hasSecondary;
}

@State() titleEl: HTMLElement;

@State() contentEl: HTMLElement;

@State() hasBack = false;

@State() hasPrimary = false;

@State() hasSecondary = false;

@State() hasContentTop = false;

Expand Down Expand Up @@ -474,6 +482,26 @@ export class Modal
//
//--------------------------------------------------------------------------

private handleHeaderSlotChange = (event: Event): void => {
this.titleEl = slotChangeGetAssignedElements<HTMLElement>(event)[0];
};

private handleContentSlotChange = (event: Event): void => {
this.contentEl = slotChangeGetAssignedElements<HTMLElement>(event)[0];
};

private handleBackSlotChange = (event: Event): void => {
this.hasBack = slotChangeHasAssignedElement(event);
};

private handlePrimarySlotChange = (event: Event): void => {
this.hasPrimary = slotChangeHasAssignedElement(event);
};

private handleSecondarySlotChange = (event: Event): void => {
this.hasSecondary = slotChangeHasAssignedElement(event);
};

private setTransitionEl = (el: HTMLDivElement): void => {
this.transitionEl = el;
};
Expand Down Expand Up @@ -533,11 +561,9 @@ export class Modal
await componentOnReady(this.el);
this.el.addEventListener("calciteModalOpen", this.openEnd);
this.opened = true;
const titleEl = getSlotted(this.el, SLOTS.header);
const contentEl = getSlotted(this.el, SLOTS.content);

this.titleId = ensureId(titleEl);
this.contentId = ensureId(contentEl);
this.titleId = ensureId(this.titleEl);
this.contentId = ensureId(this.contentEl);

if (!this.embedded) {
if (totalOpenModals === 0) {
Expand Down Expand Up @@ -582,15 +608,6 @@ export class Modal
document.documentElement.style.setProperty("overflow", initialDocumentOverflowStyle);
}

private handleMutationObserver = (): void => {
this.updateFooterVisibility();
this.updateFocusTrapElements();
};

private updateFooterVisibility = (): void => {
this.hasFooter = !!getSlotted(this.el, [SLOTS.back, SLOTS.primary, SLOTS.secondary]);
};

private updateSizeCssVars = (): void => {
this.cssWidth = getComputedStyle(this.el).getPropertyValue("--calcite-modal-width");
this.cssHeight = getComputedStyle(this.el).getPropertyValue("--calcite-modal-height");
Expand Down

0 comments on commit 879779b

Please sign in to comment.