From dae129c6a3a5ab8f157f11089dee43e7daa7286c Mon Sep 17 00:00:00 2001 From: Ryan <7389646+ryanbarlow97@users.noreply.github.com> Date: Tue, 30 Jun 2026 00:16:46 +0100 Subject: [PATCH] replace leave lobby popup with custom popup (#4449) ## Description: old: image new: image ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [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: w.o.n --- src/client/HostLobbyModal.ts | 12 +++--- src/client/JoinLobbyModal.ts | 4 +- src/client/Navigation.ts | 25 ++++++++++-- src/client/components/BaseModal.ts | 55 ++++++++++++++++++++++++--- tests/client/LeaderboardModal.test.ts | 5 ++- 5 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/client/HostLobbyModal.ts b/src/client/HostLobbyModal.ts index 878aa8084..d7f9257c9 100644 --- a/src/client/HostLobbyModal.ts +++ b/src/client/HostLobbyModal.ts @@ -531,11 +531,9 @@ export class HostLobbyModal extends BaseModal { // Clear clipboard so the host doesn't accidentally share a dead link void navigator.clipboard.writeText("").catch(() => {}); }); - if (this.modalEl) { - this.modalEl.onClose = () => { - this.close(); - }; - } + // BaseModal.firstUpdated() owns modalEl.onClose so the o-modal close path + // (backdrop / close button) runs confirmBeforeClose(). Don't override it + // here — doing so would bypass the leave-lobby confirmation. this.loadNationCount(); } @@ -552,8 +550,8 @@ export class HostLobbyModal extends BaseModal { ); } - public confirmBeforeClose(): boolean { - return confirm(translateText("host_modal.leave_confirmation")); + public confirmBeforeClose(): boolean | Promise { + return this.confirmClose(translateText("host_modal.leave_confirmation")); } protected onClose(): void { diff --git a/src/client/JoinLobbyModal.ts b/src/client/JoinLobbyModal.ts index 042e49a6c..e3270861d 100644 --- a/src/client/JoinLobbyModal.ts +++ b/src/client/JoinLobbyModal.ts @@ -336,9 +336,9 @@ export class JoinLobbyModal extends BaseModal { ); } - public confirmBeforeClose(): boolean { + public confirmBeforeClose(): boolean | Promise { if (!this.currentLobbyId) return true; - return confirm(translateText("host_modal.leave_confirmation")); + return this.confirmClose(translateText("host_modal.leave_confirmation")); } protected onClose(): void { diff --git a/src/client/Navigation.ts b/src/client/Navigation.ts index 7878155dd..f52aec437 100644 --- a/src/client/Navigation.ts +++ b/src/client/Navigation.ts @@ -79,13 +79,30 @@ export function initNavigation() { window.showPage = showPage; // Use event delegation for navigation items (they may be inside Lit components) - document.addEventListener("click", (e) => { + document.addEventListener("click", async (e) => { const target = (e.target as HTMLElement).closest( ".nav-menu-item[data-page]", ); if (target) { const pageId = (target as HTMLElement).dataset.page; - if (pageId) showPage(pageId); + if (!pageId) return; + + // showPage() closes the currently visible modal, so respect its + // close-confirmation guard first (e.g. the leave-lobby prompt). + const visibleModal = document.querySelector( + ".page-content:not(.hidden)", + ) as any; + if ( + visibleModal && + typeof visibleModal.isOpen === "function" && + visibleModal.isOpen() && + typeof visibleModal.confirmBeforeClose === "function" && + !(await visibleModal.confirmBeforeClose()) + ) { + return; + } + + showPage(pageId); } }); @@ -95,7 +112,7 @@ export function initNavigation() { const mainEl = document.querySelector("main"); if (mainEl) { - mainEl.addEventListener("click", (e: Event) => { + mainEl.addEventListener("click", async (e: Event) => { const target = e.target as HTMLElement; const isPlayPageHidden = document .getElementById("page-play") @@ -118,7 +135,7 @@ export function initNavigation() { // Check confirmation guard before closing if ( typeof openModal.confirmBeforeClose === "function" && - !openModal.confirmBeforeClose() + !(await openModal.confirmBeforeClose()) ) { return; } diff --git a/src/client/components/BaseModal.ts b/src/client/components/BaseModal.ts index 18a5efd55..a317a8937 100644 --- a/src/client/components/BaseModal.ts +++ b/src/client/components/BaseModal.ts @@ -3,6 +3,7 @@ import { property, query, state } from "lit/decorators.js"; import { modalRouter } from "../ModalRouter"; import "./baseComponents/Modal"; import type { OModalTab } from "./baseComponents/Modal"; +import "./ConfirmDialog"; /** * Static-ish configuration for the shell. @@ -39,6 +40,12 @@ export abstract class BaseModal extends LitElement { @state() protected activeTab = ""; @property({ type: Boolean }) inline = false; + // Pending close confirmation; when set, renders the inline confirm dialog. + @state() private closeConfirm: { + message: string; + resolve: (ok: boolean) => void; + } | null = null; + // Re-entrancy guard: showPage() (for inline modals) re-invokes .open() // with no args after we call it. We must not re-run onOpen(undefined) // from that nested call, which would clobber state set by the outer call. @@ -91,12 +98,30 @@ export abstract class BaseModal extends LitElement { /** * Guard called before closing via Escape key or click-outside. - * Return false to prevent the modal from closing. + * Return false (or a promise resolving false) to prevent closing. */ - public confirmBeforeClose(): boolean { + public confirmBeforeClose(): boolean | Promise { return true; } + /** + * Show a styled confirm dialog and resolve to the user's choice. Call from + * confirmBeforeClose() to gate closing behind a confirm/cancel prompt. + */ + protected confirmClose(message: string): Promise { + // Don't stack a second prompt if one is already open. + if (this.closeConfirm) return Promise.resolve(false); + return new Promise((resolve) => { + this.closeConfirm = { message, resolve }; + }); + } + + private settleCloseConfirm(ok: boolean) { + const pending = this.closeConfirm; + this.closeConfirm = null; + pending?.resolve(ok); + } + // ---- Rendering ---- createRenderRoot() { @@ -133,6 +158,14 @@ export abstract class BaseModal extends LitElement { ${headerSlot ? html`
${headerSlot}
` : null} ${body} + ${this.closeConfirm + ? html` this.settleCloseConfirm(true)} + @cancel=${() => this.settleCloseConfirm(false)} + >` + : null} `; } @@ -199,6 +232,9 @@ export abstract class BaseModal extends LitElement { } public close(args?: Record): void { + // If closing was triggered elsewhere while a confirm prompt is pending, + // dismiss it (removes the dialog and resolves the awaiting guard as false). + this.settleCloseConfirm(false); this.unregisterEscapeHandler(); this.onClose(args); @@ -234,9 +270,13 @@ export abstract class BaseModal extends LitElement { protected firstUpdated(): void { if (this.modalEl) { - this.modalEl.onClose = () => { + this.modalEl.onClose = async () => { if (this.isModalOpen) { - if (!this.confirmBeforeClose()) { + const confirmed = await this.confirmBeforeClose(); + // Bail if a parallel close() settled things while we awaited — + // otherwise we'd re-open an already-closed modal. + if (!this.isModalOpen) return; + if (!confirmed) { // Re-open the underlying o-modal since it already closed itself this.modalEl?.open(); return; @@ -248,14 +288,17 @@ export abstract class BaseModal extends LitElement { } disconnectedCallback() { + this.settleCloseConfirm(false); this.unregisterEscapeHandler(); super.disconnectedCallback(); } - private handleKeyDown = (e: KeyboardEvent) => { + private handleKeyDown = async (e: KeyboardEvent) => { if (e.key === "Escape" && this.isModalOpen) { e.preventDefault(); - if (!this.confirmBeforeClose()) { + const confirmed = await this.confirmBeforeClose(); + // Bail if a parallel close() already closed us while we awaited. + if (!confirmed || !this.isModalOpen) { return; } this.close(); diff --git a/tests/client/LeaderboardModal.test.ts b/tests/client/LeaderboardModal.test.ts index 2bef31f1b..fd3f6362f 100644 --- a/tests/client/LeaderboardModal.test.ts +++ b/tests/client/LeaderboardModal.test.ts @@ -303,7 +303,7 @@ describe("LeaderboardModal", () => { expect(modal.tagName.toLowerCase()).toBe("leaderboard-modal"); }); - it("should close on Escape when open", () => { + it("should close on Escape when open", async () => { const mockModalEl = { open: vi.fn(), close: vi.fn() }; Object.defineProperty(modal, "modalEl", { get: () => mockModalEl, @@ -317,6 +317,9 @@ describe("LeaderboardModal", () => { ); window.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })); + // handleKeyDown awaits confirmBeforeClose() before closing, so the close + // is deferred to a later microtask — flush it before asserting. + await new Promise((resolve) => setTimeout(resolve, 0)); expect((modal as unknown as { isModalOpen: boolean }).isModalOpen).toBe( false, );