Close the settings dialog and honour Back off the live URL

This commit is contained in:
Anthony Stirling
2026-08-19 18:54:02 +01:00
parent bd01610ed6
commit 108badbe5e
3 changed files with 73 additions and 11 deletions
@@ -54,6 +54,21 @@ interface AppConfigModalProps {
hiddenSectionKeys?: NavKey[];
}
/**
* Whether this session has a history entry behind the current one.
*
* react-router keeps its own entry index in `history.state.idx`, so reading it
* live is the non-stale equivalent of the `location.key === "default"` test
* this used to do ("default" = first entry, nothing to pop back to). It has to
* be live: `location` lags behind a pending transition, and intra-modal tab
* switches rewrite the URL through `history.replaceState`, which react-router
* never observes at all.
*/
const canUnwindHistory = (): boolean => {
const idx = (window.history.state as { idx?: number } | null)?.idx;
return typeof idx === "number" && idx > 0;
};
// Extract section from URL path (e.g., /settings/people -> people)
const getSectionFromPath = (pathname: string): NavKey | null => {
const match = pathname.match(/\/settings\/([^/]+)/);
@@ -89,10 +104,6 @@ const AppConfigModalInner: React.FC<AppConfigModalProps> = ({
const licenseAlert = useLicenseAlert();
const { confirmIfDirty } = useUnsavedChanges();
const closeButtonRef = useRef<HTMLButtonElement>(null);
// Whether this modal pushed its own /settings entry, so close knows there is
// one to pop. `location.key` can't answer that: react-router defers location
// updates through a transition and may still read the pre-open entry.
const pushedSettingsEntry = useRef(false);
// Sync active state with URL path. Runs on open, on external URL changes,
// and on the redirect path below - NOT on intra-modal tab clicks, because
@@ -151,7 +162,6 @@ const AppConfigModalInner: React.FC<AppConfigModalProps> = ({
withBasePath(`/settings/${key}`),
);
} else {
pushedSettingsEntry.current = true;
navigate(`/settings/${key}`);
}
},
@@ -235,8 +245,7 @@ const AppConfigModalInner: React.FC<AppConfigModalProps> = ({
// the pre-open path here and skip the unwind entirely - leaving the URL on
// /settings/* so the modal immediately re-opens.
if (urlSync && isInSettings()) {
if (pushedSettingsEntry.current) {
pushedSettingsEntry.current = false;
if (canUnwindHistory()) {
navigate(-1);
} else {
// Deep link or refresh straight into /settings: nothing to pop to.
+9 -4
View File
@@ -105,11 +105,16 @@ export default function HomePage() {
// Open the config modal whenever the URL is /settings/* (e.g. from the admin
// tour's openConfigModal action which navigates to /settings/overview).
//
// Read the live URL rather than `location.pathname`: react-router defers
// location updates through a transition, so under load it can still hold the
// pre-navigation path and re-open a modal the user just closed.
// Both halves read the live URL rather than `location.pathname`, and popstate
// is subscribed to directly: react-router defers location updates through a
// transition, so `location` can still hold the pre-navigation path. Deriving
// from it re-opens a modal the user just closed, and depending on it alone
// misses Back entirely when the push it should have committed never landed.
useEffect(() => {
setConfigModalOpen(isInSettings());
const syncFromUrl = () => setConfigModalOpen(isInSettings());
syncFromUrl();
window.addEventListener("popstate", syncFromUrl);
return () => window.removeEventListener("popstate", syncFromUrl);
}, [location.pathname]);
useEffect(() => {
@@ -233,6 +233,54 @@ test.describe("Settings dialog", () => {
.toBe(originPath);
});
test("browser Back closes the dialog and returns to the origin URL", async ({
page,
}) => {
await page.goto("/compress", { waitUntil: "domcontentloaded" });
const originPath = new URL(page.url()).pathname;
const dialog = await openSettings(page);
await dialog.locator('[data-tour="admin-general-nav"]').first().click();
await page.waitForURL(/\/settings\/general/, { timeout: 5_000 });
// Back has to close the modal off the URL alone. react-router's location
// may never have committed the push that opened it, so an effect keyed
// only on `location.pathname` sees no change here and leaves it open.
await page.goBack();
await expect(
page.locator(".mantine-Modal-content").first(),
).not.toBeVisible({ timeout: 5_000 });
await expect
.poll(() => new URL(page.url()).pathname, { timeout: 5_000 })
.toBe(originPath);
});
test("close returns to origin URL when settings was opened from super search", async ({
page,
}) => {
// Super search pushes /settings/<section> itself, without going through
// the modal's own tab nav. Close still has to pop that entry rather than
// fall back to the editor home, or the user loses the page they were on.
await page.goto("/compress", { waitUntil: "domcontentloaded" });
const originPath = new URL(page.url()).pathname;
const input = page.locator("#super-search-input");
await expect(input).toBeVisible({ timeout: 5_000 });
await input.click();
await input.fill("general");
await page
.getByRole("option", { name: /General/ })
.first()
.click();
await expect(page.locator(".modal-container")).toBeVisible({
timeout: 5_000,
});
await page.waitForURL(/\/settings\/general/, { timeout: 5_000 });
await closeSettings(page);
await expect
.poll(() => new URL(page.url()).pathname, { timeout: 5_000 })
.toBe(originPath);
});
test("config sub-sections (System / Features / Endpoints / API Keys) are reachable when present", async ({
page,
}) => {