mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-07-23 18:20:52 +00:00
fix: defer closed modal rendering (#4670)
> **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 `<o-modal>` 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
This commit is contained in:
@@ -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`
|
||||
<o-modal
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
import { html, TemplateResult } from "lit";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { BaseModal } from "../../src/client/components/BaseModal";
|
||||
|
||||
class LazyBodyTestModal extends BaseModal {
|
||||
bodyRenderCount = 0;
|
||||
headerRenderCount = 0;
|
||||
|
||||
protected renderHeaderSlot(): TemplateResult {
|
||||
this.headerRenderCount++;
|
||||
return html`<span data-testid="header">Header</span>`;
|
||||
}
|
||||
|
||||
protected renderBody(): TemplateResult {
|
||||
this.bodyRenderCount++;
|
||||
return html`<span data-testid="body">Body</span>`;
|
||||
}
|
||||
}
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user