From c96b682e18b4c18112b3df4fa5f96db6eb6a66fd Mon Sep 17 00:00:00 2001 From: Josh Harris Date: Wed, 22 Jul 2026 20:47:41 +0100 Subject: [PATCH] fix: defer closed modal rendering (#4670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > **Before opening a PR:** discuss new features on [Discord](https://discord.gg/K9zernJB5z) first, and file bugs or small improvements as [issues](https://github.com/openfrontio/OpenFrontIO/issues/new/choose). You must be assigned to an `approved` issue — unsolicited PRs will be auto-closed. ## Description: Closed `BaseModal` instances rendered their complete headers and bodies during page startup. A hidden cosmetics modal consequently generated more than 1,200 pattern-preview data URLs in one Lit microtask checkpoint, blocking the renderer for 11–30 seconds and delaying unrelated promise continuations such as the clan-tag ownership spinner. This hotfix keeps the lightweight `` shell mounted, but defers its header and body until the modal opens and unmounts that content again on close. It does not change the cosmetics UI. Validation: - `vitest run tests/client/BaseModal.test.ts` - `npm run build-prod` ## Please complete the following: - [x] I have added screenshots for all UI updates (no UI changes) - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file (no new user-facing text) - [x] I have added relevant tests to the test directory ## Please put your Discord username so you can be contacted if a bug or regression is found: jish --- src/client/components/BaseModal.ts | 11 +++-- tests/client/BaseModal.test.ts | 69 +++++++++++++++++++++++++++ tests/client/LeaderboardModal.test.ts | 2 + 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/client/BaseModal.test.ts diff --git a/src/client/components/BaseModal.ts b/src/client/components/BaseModal.ts index 18a5efd55..22a3302c4 100644 --- a/src/client/components/BaseModal.ts +++ b/src/client/components/BaseModal.ts @@ -1,4 +1,4 @@ -import { html, LitElement, TemplateResult } from "lit"; +import { html, LitElement, nothing, TemplateResult } from "lit"; import { property, query, state } from "lit/decorators.js"; import { modalRouter } from "../ModalRouter"; import "./baseComponents/Modal"; @@ -115,8 +115,13 @@ export abstract class BaseModal extends LitElement { render(): TemplateResult { const cfg = this.modalConfig(); const tabs = cfg.tabs ?? []; - const body = this.renderBody(this.activeTab); - const headerSlot = this.renderHeaderSlot(); + // Keep the lightweight modal shell mounted so open() can address it, but + // defer potentially expensive modal contents until they are visible. + const shouldRenderContent = this.inline || this.isModalOpen; + const body = shouldRenderContent + ? this.renderBody(this.activeTab) + : nothing; + const headerSlot = shouldRenderContent ? this.renderHeaderSlot() : null; return html` Header`; + } + + protected renderBody(): TemplateResult { + this.bodyRenderCount++; + return html`Body`; + } +} + +if (!customElements.get("lazy-body-test-modal")) { + customElements.define("lazy-body-test-modal", LazyBodyTestModal); +} + +describe("BaseModal lazy rendering", () => { + let modal: LazyBodyTestModal | undefined; + + afterEach(() => { + modal?.remove(); + modal = undefined; + }); + + it("does not render modal content until opened", async () => { + modal = document.createElement("lazy-body-test-modal") as LazyBodyTestModal; + document.body.appendChild(modal); + await modal.updateComplete; + + const modalShell = modal.querySelector("o-modal"); + expect(modalShell).not.toBeNull(); + expect(modal.bodyRenderCount).toBe(0); + expect(modal.headerRenderCount).toBe(0); + expect(modal.querySelector('[data-testid="body"]')).toBeNull(); + + modal.open(); + await modal.updateComplete; + + expect(modal.bodyRenderCount).toBe(1); + expect(modal.headerRenderCount).toBe(1); + expect(modal.querySelector('[data-testid="body"]')).not.toBeNull(); + + modal.close(); + await modal.updateComplete; + + expect(modal.querySelector('[data-testid="body"]')).toBeNull(); + expect(modal.querySelector('[data-testid="header"]')).toBeNull(); + expect(modal.querySelector("o-modal")).toBe(modalShell); + }); + + it("renders inline content without being opened", async () => { + modal = document.createElement("lazy-body-test-modal") as LazyBodyTestModal; + modal.setAttribute("inline", ""); + document.body.appendChild(modal); + await modal.updateComplete; + + expect(modal.bodyRenderCount).toBe(1); + expect(modal.headerRenderCount).toBe(1); + expect(modal.querySelector('[data-testid="body"]')).not.toBeNull(); + expect(modal.querySelector('[data-testid="header"]')).not.toBeNull(); + }); +}); diff --git a/tests/client/LeaderboardModal.test.ts b/tests/client/LeaderboardModal.test.ts index 2bef31f1b..a7e9ec748 100644 --- a/tests/client/LeaderboardModal.test.ts +++ b/tests/client/LeaderboardModal.test.ts @@ -133,6 +133,7 @@ describe("LeaderboardModal", () => { customElements.define("leaderboard-modal", LeaderboardModal); } modal = document.createElement("leaderboard-modal") as LeaderboardModal; + modal.inline = true; document.body.appendChild(modal); await modal.updateComplete; }); @@ -304,6 +305,7 @@ describe("LeaderboardModal", () => { }); it("should close on Escape when open", () => { + modal.inline = false; const mockModalEl = { open: vi.fn(), close: vi.fn() }; Object.defineProperty(modal, "modalEl", { get: () => mockModalEl,