From d953cd20fa2de9c6ddce82d90708661e3e5f8887 Mon Sep 17 00:00:00 2001 From: Nameless 7778777 <7778777@7778777.online> Date: Wed, 5 Aug 2026 16:29:01 +0200 Subject: [PATCH 1/3] Close the image lightbox on back button/gesture instead of leaving it stuck open On Android, pressing the back button while the lightbox is open navigated the underlying page while the lightbox overlay stayed on screen, since it never integrated with browser history. It now pushes a history entry when opened and closes itself on popstate; the router skips reacting to that navigation so the page underneath doesn't change. --- src/js/components/lightbox-image-group.js | 14 +++- src/js/router.js | 20 +++++ .../components/lightbox-image-group.test.js | 77 +++++++++++++++++++ tests/unit/specs/router.test.js | 44 ++++++++++- 4 files changed, 153 insertions(+), 2 deletions(-) diff --git a/src/js/components/lightbox-image-group.js b/src/js/components/lightbox-image-group.js index 59cd7feb..35294c0b 100644 --- a/src/js/components/lightbox-image-group.js +++ b/src/js/components/lightbox-image-group.js @@ -4,6 +4,7 @@ import { ImageLoader } from "/js/utils.js"; import { chevronLeftIconTemplate } from "/js/templates/icons/chevronLeft.template.js"; import { chevronRightIconTemplate } from "/js/templates/icons/chevronRight.template.js"; import { closeIconTemplate } from "/js/templates/icons/closeIcon.template.js"; +import { pushOverlayHistoryEntry } from "/js/router.js"; class LightboxDialog extends Component { connectedCallback() { @@ -120,12 +121,23 @@ class LightboxDialog extends Component { this.isOpen = true; this.handleKeyDown = this.handleKeyDown.bind(this); document.addEventListener("keydown", this.handleKeyDown); + this.handlePopState = this.handlePopState.bind(this); + window.addEventListener("popstate", this.handlePopState); + pushOverlayHistoryEntry(); this.render(); } - close() { + handlePopState() { + this.close({ viaPopState: true }); + } + + close({ viaPopState = false } = {}) { document.body.style.overflow = ""; document.removeEventListener("keydown", this.handleKeyDown); + window.removeEventListener("popstate", this.handlePopState); + if (!viaPopState) { + window.history.back(); + } this.isOpen = false; this._imageLoader.abort(); this.render(); diff --git a/src/js/router.js b/src/js/router.js index 08250ced..ade86422 100644 --- a/src/js/router.js +++ b/src/js/router.js @@ -4,6 +4,22 @@ import { BoundedMap } from "/js/utils.js"; const MAX_PAGES = 5; +// Lets an overlay (e.g. the image lightbox) claim the next back navigation +// to close itself instead of the router navigating the underlying page. +// Call once when the overlay opens; the pushed entry is consumed either by +// a real back button press or by the overlay's own history.back() when +// it's dismissed some other way (see the popstate listener below). +let activeOverlayHistoryEntries = 0; + +export function pushOverlayHistoryEntry() { + activeOverlayHistoryEntries++; + window.history.pushState( + { ...window.history.state }, + "", + window.location.href, + ); +} + // Browsers fire auxclick rather than click for the middle button, so // we need to manually dispatch a click event for it function bindMiddleClickRedispatch() { @@ -118,6 +134,10 @@ export class Router extends EventEmitter { }); // on back button, go back to the previous page window.addEventListener("popstate", async (e) => { + if (activeOverlayHistoryEntries > 0) { + activeOverlayHistoryEntries--; + return; + } this.emit("navigate"); await this.load(window.location.pathname + window.location.search, { isBack: true, diff --git a/tests/unit/specs/components/lightbox-image-group.test.js b/tests/unit/specs/components/lightbox-image-group.test.js index 23f484ea..70b6c387 100644 --- a/tests/unit/specs/components/lightbox-image-group.test.js +++ b/tests/unit/specs/components/lightbox-image-group.test.js @@ -334,6 +334,83 @@ describe("lightbox-image-group", () => { }); }); + describe("LightboxDialog - back button / history integration", () => { + it("should push a history entry when opened", () => { + const originalLength = window.history.length; + const element = document.createElement("lightbox-dialog"); + element.images = [createTestImage("test.jpg", "Test")]; + document.body.appendChild(element); + element.open(); + + assert.deepEqual(window.history.length, originalLength + 1); + + element.close(); + }); + + it("should close when a popstate event fires while open (e.g. Android back button)", () => { + const element = document.createElement("lightbox-dialog"); + element.images = [createTestImage("test.jpg", "Test")]; + document.body.appendChild(element); + element.open(); + + window.dispatchEvent(new Event("popstate")); + + assert.deepEqual(element.isOpen, false); + }); + + it("should not call history.back() again when closed via popstate", () => { + const element = document.createElement("lightbox-dialog"); + element.images = [createTestImage("test.jpg", "Test")]; + document.body.appendChild(element); + element.open(); + + const originalBack = window.history.back; + let backCalls = 0; + window.history.back = () => { + backCalls++; + }; + try { + window.dispatchEvent(new Event("popstate")); + } finally { + window.history.back = originalBack; + } + + assert.deepEqual(backCalls, 0); + }); + + it("should call history.back() to release its pushed entry when closed some other way", () => { + const element = document.createElement("lightbox-dialog"); + element.images = [createTestImage("test.jpg", "Test")]; + document.body.appendChild(element); + element.open(); + + const originalBack = window.history.back; + let backCalls = 0; + window.history.back = () => { + backCalls++; + }; + try { + element.close(); + } finally { + window.history.back = originalBack; + } + + assert.deepEqual(backCalls, 1); + }); + + it("should stop listening for popstate after closing", () => { + const element = document.createElement("lightbox-dialog"); + element.images = [createTestImage("test.jpg", "Test")]; + document.body.appendChild(element); + element.open(); + element.close(); + + // A stray popstate after close should not throw or reopen the dialog. + window.dispatchEvent(new Event("popstate")); + assert.deepEqual(element.isOpen, false); + }); + }); + describe("LightboxDialog - keyboard navigation", () => { it("should close on Escape key", () => { const element = document.createElement("lightbox-dialog"); diff --git a/tests/unit/specs/router.test.js b/tests/unit/specs/router.test.js index cb417aa6..794f3786 100644 --- a/tests/unit/specs/router.test.js +++ b/tests/unit/specs/router.test.js @@ -1,6 +1,6 @@ import { describe, it, beforeEach, afterEach, mock } from "node:test"; import assert from "node:assert/strict"; -import { Router, Layout } from "/js/router.js"; +import { Router, Layout, pushOverlayHistoryEntry } from "/js/router.js"; class TestLayout extends Layout { constructor() { @@ -322,6 +322,48 @@ describe("popstate", () => { window.history.replaceState(originalState, "", originalPath); } }); + + it("should not emit navigate when popstate fires while an overlay history entry is pending", async () => { + const originalState = window.history.state; + const originalPath = + window.location.pathname + window.location.search + window.location.hash; + const { router, popstateHandler } = createRouterWithPopstateHandler(); + mountRouter(router); + + const listener = mock.fn(); + router.on("navigate", listener); + + try { + // Mirrors what the image lightbox does when it opens. + pushOverlayHistoryEntry(); + await popstateHandler(new Event("popstate")); + + assert.deepEqual(listener.mock.callCount(), 0); + } finally { + window.history.replaceState(originalState, "", originalPath); + } + }); + + it("should resume normal navigation on the popstate after an overlay entry is consumed", async () => { + const originalState = window.history.state; + const originalPath = + window.location.pathname + window.location.search + window.location.hash; + const { router, popstateHandler } = createRouterWithPopstateHandler(); + mountRouter(router); + + const listener = mock.fn(); + router.on("navigate", listener); + + try { + pushOverlayHistoryEntry(); + await popstateHandler(new Event("popstate")); // consumes the overlay entry + await popstateHandler(new Event("popstate")); // a real back navigation + + assert.deepEqual(listener.mock.callCount(), 1); + } finally { + window.history.replaceState(originalState, "", originalPath); + } + }); }); describe("load", () => { -- 2.54.0 From b29d2563657b84d1595eb87e6bd160b189b404ec Mon Sep 17 00:00:00 2001 From: Nameless 7778777 <7778777@7778777.online> Date: Wed, 5 Aug 2026 16:59:28 +0200 Subject: [PATCH 2/3] Track the pending overlay history entry via a DOM marker instead of a counter The in-memory counter was module-level state shared by every Router instance, including the dozens the test suite creates. Since the parallel test runner shares one process (and module cache) across files, a lightbox test that opens without closing left the counter incremented forever, causing unrelated popstate tests elsewhere to spuriously skip navigation. A marker appended to document.body is scoped to the live DOM instead, so it can't outlive a test's own cleanup. --- src/js/components/lightbox-image-group.js | 24 +++++++++++++++++--- src/js/router.js | 27 +++++++++++++++-------- tests/unit/specs/router.test.js | 13 ++++++----- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/js/components/lightbox-image-group.js b/src/js/components/lightbox-image-group.js index 35294c0b..6bd1800c 100644 --- a/src/js/components/lightbox-image-group.js +++ b/src/js/components/lightbox-image-group.js @@ -119,23 +119,41 @@ class LightboxDialog extends Component { open() { document.body.style.overflow = "hidden"; this.isOpen = true; + this._closed = false; this.handleKeyDown = this.handleKeyDown.bind(this); document.addEventListener("keydown", this.handleKeyDown); this.handlePopState = this.handlePopState.bind(this); window.addEventListener("popstate", this.handlePopState); - pushOverlayHistoryEntry(); + this._historyMarker = pushOverlayHistoryEntry(); this.render(); } handlePopState() { + if (this._closed) { + // Just confirming the history.back() we triggered ourselves when + // closing some other way; release the marker that told the router + // to ignore this popstate. + window.removeEventListener("popstate", this.handlePopState); + this._historyMarker?.remove(); + this._historyMarker = null; + return; + } this.close({ viaPopState: true }); } close({ viaPopState = false } = {}) { + if (this._closed) return; + this._closed = true; document.body.style.overflow = ""; document.removeEventListener("keydown", this.handleKeyDown); - window.removeEventListener("popstate", this.handlePopState); - if (!viaPopState) { + if (viaPopState) { + window.removeEventListener("popstate", this.handlePopState); + this._historyMarker?.remove(); + this._historyMarker = null; + } else { + // Leave the popstate listener attached: handlePopState() above + // releases the marker once this history.back() actually consumes + // the entry pushed in open(). window.history.back(); } this.isOpen = false; diff --git a/src/js/router.js b/src/js/router.js index ade86422..9fea1d2c 100644 --- a/src/js/router.js +++ b/src/js/router.js @@ -6,18 +6,30 @@ const MAX_PAGES = 5; // Lets an overlay (e.g. the image lightbox) claim the next back navigation // to close itself instead of the router navigating the underlying page. -// Call once when the overlay opens; the pushed entry is consumed either by -// a real back button press or by the overlay's own history.back() when -// it's dismissed some other way (see the popstate listener below). -let activeOverlayHistoryEntries = 0; +// Call once when the overlay opens; it pushes a history entry and tags the +// document with a marker element for as long as that entry hasn't been +// consumed yet. The caller is responsible for removing the returned marker +// once the entry is actually popped (either by a real back button press or +// by the overlay's own history.back() when it's dismissed some other way) +// — see the popstate listener below, which just checks for the marker's +// presence. Tracking this via a DOM node rather than in-memory state means +// a test that opens an overlay and never tears it down can't leave stale +// state behind for unrelated code: the marker disappears the moment +// something clears the document (e.g. a test's own cleanup), instead of +// silently lingering for the lifetime of the module. +const OVERLAY_HISTORY_PENDING_SELECTOR = "[data-overlay-history-pending]"; export function pushOverlayHistoryEntry() { - activeOverlayHistoryEntries++; + const marker = document.createElement("span"); + marker.hidden = true; + marker.setAttribute("data-overlay-history-pending", ""); + document.body.appendChild(marker); window.history.pushState( { ...window.history.state }, "", window.location.href, ); + return marker; } // Browsers fire auxclick rather than click for the middle button, so @@ -134,10 +146,7 @@ export class Router extends EventEmitter { }); // on back button, go back to the previous page window.addEventListener("popstate", async (e) => { - if (activeOverlayHistoryEntries > 0) { - activeOverlayHistoryEntries--; - return; - } + if (document.querySelector(OVERLAY_HISTORY_PENDING_SELECTOR)) return; this.emit("navigate"); await this.load(window.location.pathname + window.location.search, { isBack: true, diff --git a/tests/unit/specs/router.test.js b/tests/unit/specs/router.test.js index 794f3786..7fe1852a 100644 --- a/tests/unit/specs/router.test.js +++ b/tests/unit/specs/router.test.js @@ -333,18 +333,19 @@ describe("popstate", () => { const listener = mock.fn(); router.on("navigate", listener); + // Mirrors what the image lightbox does when it opens. + const marker = pushOverlayHistoryEntry(); try { - // Mirrors what the image lightbox does when it opens. - pushOverlayHistoryEntry(); await popstateHandler(new Event("popstate")); assert.deepEqual(listener.mock.callCount(), 0); } finally { + marker.remove(); window.history.replaceState(originalState, "", originalPath); } }); - it("should resume normal navigation on the popstate after an overlay entry is consumed", async () => { + it("should resume normal navigation once the overlay releases its marker", async () => { const originalState = window.history.state; const originalPath = window.location.pathname + window.location.search + window.location.hash; @@ -354,9 +355,11 @@ describe("popstate", () => { const listener = mock.fn(); router.on("navigate", listener); + const marker = pushOverlayHistoryEntry(); try { - pushOverlayHistoryEntry(); - await popstateHandler(new Event("popstate")); // consumes the overlay entry + await popstateHandler(new Event("popstate")); // overlay consumes this one + // Mirrors what the lightbox does once its own history.back() resolves. + marker.remove(); await popstateHandler(new Event("popstate")); // a real back navigation assert.deepEqual(listener.mock.callCount(), 1); -- 2.54.0 From 345c0e90d957108d49a9d43e7788b3bb79f31e62 Mon Sep 17 00:00:00 2001 From: Nameless 7778777 <7778777@7778777.online> Date: Thu, 6 Aug 2026 13:11:50 +0200 Subject: [PATCH 3/3] Convert lightbox to a native dialog instead of custom history tracking Per review feedback on #34: replace the router-level history-tracking mechanism (pushOverlayHistoryEntry, the popstate special-case) with the data-dialog-wrapper convention every other dialog in the app already uses. app.js already closes all open dialog[open] elements on every navigation, including back-button navigation, so converting the lightbox's plain