Give a modal history entry back when navigating out of it instead of replacing it (#631)

AGENTS.md

@@ -198,8 +198,9 @@ return a `Command`, so `.then(publish)` is usually all you need.
 - Import from `app/modal.ts` or `app/toast.ts`
 - Pass component objects with parameters
 - Use `$state.snapshot` if calling component might unmount
-- Navigate with `navigate` from `app/modal.ts` rather than `goto` — an open modal owns the current
-  history entry, so a navigation that drops it replaces that entry instead of stacking on it
+- Navigate with `navigate` from `app/modal.ts` rather than `goto` — an open modal owns a history
+  entry, and a navigation that drops the modal gives that entry back before it pushes its own.
+  A plain `<a>` inside a modal goes the same way, through `ModalContainer`'s `beforeNavigate`
 - Pass `keepModal` to `navigate` to change the page under a modal and leave it open
 
 ## Development Workflow

e2e/specs/routing.spec.ts

@@ -3,7 +3,18 @@ import type {TrustedEvent} from "@welshman/util"
 import {RelayMessageType} from "@welshman/net"
 import {Article} from "@welshman/domain"
 import type {Page} from "@playwright/test"
-import {expect, getTranscript, pathPattern, roomPath, spacePath, test, users} from "../harness"
+import {
+  dialog,
+  expect,
+  getTranscript,
+  message,
+  pathPattern,
+  profilePath,
+  roomPath,
+  spacePath,
+  test,
+  users,
+} from "../harness"
 
 // Every page in a space renders one PageContent, so the number added to the document is the number
 // of times the page was built.
@@ -150,6 +161,7 @@ test("takes over the space menu's history entry when you navigate out of it", as
     space.room("garden", {name: "Space Garden"})
     space.join(user.alice, "lounge")
     space.join(user.alice, "garden")
+    space.message(user.alice, "lounge", "the lounge is this way")
   })
 
   const space = scenario.space("space")
@@ -159,18 +171,23 @@ test("takes over the space menu's history entry when you navigate out of it", as
 
   const drawer = page.locator(".drawer")
 
+  await expect(message(page, "the lounge is this way")).toBeVisible()
+
   await page.getByRole("button", {name: "Open space menu"}).click()
   await drawer.getByRole("link", {name: "Space Garden"}).click()
 
   await expect(page).toHaveURL(new RegExp(`${roomPath(space.url, "garden")}$`))
   await expect(drawer).toHaveCount(0)
 
-  // The menu held the entry the room it opened now holds, so one step back is the room it opened
-  // over. Stacked instead, this lands on the menu again.
+  // The menu gave its entry back and the room pushed its own, so one step back is the room the
+  // menu was opened over. Stacked instead, this lands on the menu again. The room's content is
+  // asserted as well as the url, since a back that only rewrites the url passes every assertion
+  // about the address.
   await page.goBack()
 
   await expect(page).toHaveURL(new RegExp(`${roomPath(space.url, "lounge")}$`))
   await expect(drawer).toHaveCount(0)
+  await expect(message(page, "the lounge is this way")).toBeVisible()
 })
 
 test("switches spaces inside the space menu, and out of one it has a page for", async ({
@@ -299,3 +316,38 @@ test("builds a page once when it opens and again when its params change", async
 
   await expectPageBuilds(page, 2)
 })
+
+test("goes back to the room a profile modal was opened over", async ({seed, as}) => {
+  const scenario = await seed(({relay, user}) => {
+    const space = relay("space")
+
+    space.room("lounge", {name: "Space Lounge"})
+    space.join(user.alice, "lounge")
+    space.join(user.bob, "lounge")
+    space.profile(user.bob, {name: "Bob Barnacle"})
+    space.message(user.bob, "lounge", "anyone seen the anchor")
+  })
+
+  const space = scenario.space("space")
+  const page = await as(users.alice, roomPath(space.url, "lounge"))
+
+  await expect(message(page, "anyone seen the anchor")).toBeVisible()
+
+  // The whole message is a button too, and its accessible name carries the author's, so the
+  // name on its own is the one that opens the profile.
+  await message(page, "anyone seen the anchor")
+    .getByRole("button", {name: "Bob Barnacle", exact: true})
+    .click()
+  await expect(dialog(page, "Profile details")).toBeVisible()
+
+  await page.getByRole("button", {name: "View Full Profile"}).click()
+  await expect(page).toHaveURL(new RegExp(`${profilePath(users.bob.pubkey)}$`))
+
+  // The modal gave its entry back before the profile page pushed its own, so one step back is the
+  // room. Replacing that entry instead left SvelteKit with the navigation index the room already
+  // had, and it answered the back by writing the url without building the page again.
+  await page.goBack()
+
+  await expect(page).toHaveURL(new RegExp(`${roomPath(space.url, "lounge")}$`))
+  await expect(message(page, "anyone seen the anchor")).toBeVisible()
+})

src/app/components/ModalContainer.svelte

@@ -1,9 +1,20 @@
 <script lang="ts">
   import type {Component, ComponentProps} from "svelte"
   import {mount, unmount, untrack} from "svelte"
+  import {beforeNavigate} from "$app/navigation"
   import Drawer from "@lib/components/Drawer.svelte"
   import Dialog from "@lib/components/Dialog.svelte"
-  import {getModal, getModalStack, popModal} from "@app/modal"
+  import {getModal, getModalStack, navigate, popModal} from "@app/modal"
+
+  // A link inside a modal is SvelteKit's to handle, and it would stack the page it opens on
+  // top of the modal's own history entry. Hand it to `navigate`, which gives that entry back
+  // first.
+  beforeNavigate(navigation => {
+    if (navigation.type === "link" && navigation.to && getModalStack().length > 0) {
+      navigation.cancel()
+      navigate(navigation.to.url.href)
+    }
+  })
 
   const closeModal = () => {
     const modal = getModal()
@@ -80,4 +91,4 @@
 
 <svelte:window onkeydown={onKeyDown} />
 
-<div bind:this={element} data-sveltekit-replacestate="true"></div>
+<div bind:this={element}></div>

src/app/components/ProfileDetail.svelte

@@ -42,7 +42,7 @@
 
   const back = () => history.back()
 
-  const viewProfile = () => navigate(makeProfilePath(pubkey), {replaceState: true})
+  const viewProfile = () => navigate(makeProfilePath(pubkey))
 
   const sendMessage = () => {
     popModal()

src/app/modal.ts

@@ -34,15 +34,39 @@ export const getModal = () => last(getModalStack())
 
 export type NavigateOptions = Parameters<typeof goto>[1] & {keepModal?: boolean}
 
-// An open modal owns the current history entry, so a navigation that drops it takes that entry over
-export const navigate = (path: string, {keepModal, ...options}: NavigateOptions = {}) => {
+const popHistory = () =>
+  new Promise(resolve => {
+    addEventListener("popstate", resolve, {once: true})
+
+    history.back()
+  })
+
+// Each open modal owns a history entry, and a navigation that drops them gives those entries back
+// rather than replacing them. SvelteKit reuses its navigation index for a `goto` that replaces, and
+// a back out of the new page would then match the entry underneath and update the url without
+// rendering it.
+const dropModalEntries = async () => {
+  let dropped = false
+
+  while (getModalStack().length > 0) {
+    await popHistory()
+
+    dropped = true
+  }
+
+  return dropped
+}
+
+export const navigate = async (path: string, {keepModal, ...options}: NavigateOptions = {}) => {
   const ids = page.state.modals ?? []
 
   if (keepModal && ids.length > 0) {
     return goto(path, {...options, state: {modals: ids}, replaceState: true})
   }
 
-  return goto(path, {...options, replaceState: options.replaceState || ids.length > 0})
+  const dropped = await dropModalEntries()
+
+  return goto(path, {...options, replaceState: options.replaceState && !dropped})
 }
 
 export const pushModal = (