diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c0e7942..c7cc512e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,7 @@ generated user module: ### Bug fixes * Fix LiveComponents in nested LiveViews not updating under certain conditions ([#3626](https://github.com/phoenixframework/phoenix_live_view/issues/3626)) * Fix client-side hooks not being cleared properly ([#3628](https://github.com/phoenixframework/phoenix_live_view/issues/3628)) +* Fix regression where browser back/forward buttons used `patch` instead of `navigate`, failing to update the page ([#3529](https://github.com/phoenixframework/phoenix_live_view/issues/3529)) ## 1.0.2 (2025-01-09) diff --git a/assets/js/phoenix_live_view/live_socket.js b/assets/js/phoenix_live_view/live_socket.js index feaa5884e..b664307f3 100644 --- a/assets/js/phoenix_live_view/live_socket.js +++ b/assets/js/phoenix_live_view/live_socket.js @@ -695,7 +695,7 @@ export default class LiveSocket { }) window.addEventListener("popstate", event => { if(!this.registerNewLocation(window.location)){ return } - let {type, backType, id, root, scroll, position} = event.state || {} + let {type, backType, id, scroll, position} = event.state || {} let href = window.location.href // Compare positions to determine direction @@ -709,15 +709,11 @@ export default class LiveSocket { DOM.dispatchEvent(window, "phx:navigate", {detail: {href, patch: type === "patch", pop: true, direction: isForward ? "forward" : "backward"}}) this.requestDOMUpdate(() => { + const callback = () => { this.maybeScroll(scroll) } if(this.main.isConnected() && (type === "patch" && id === this.main.id)){ - this.main.pushLinkPatch(event, href, null, () => { - this.maybeScroll(scroll) - }) + this.main.pushLinkPatch(event, href, null, callback) } else { - this.replaceMain(href, null, () => { - if(root){ this.replaceRootHistory() } - this.maybeScroll(scroll) - }) + this.replaceMain(href, null, callback) } }) }, false) @@ -838,15 +834,6 @@ export default class LiveSocket { }) } - replaceRootHistory(){ - Browser.pushState("replace", { - root: true, - type: "patch", - id: this.main.id, - position: this.currentHistoryPosition // Preserve current position - }) - } - registerNewLocation(newLocation){ let {pathname, search} = this.currentLocation if(pathname + search === newLocation.pathname + newLocation.search){ diff --git a/assets/js/phoenix_live_view/view.js b/assets/js/phoenix_live_view/view.js index b74084a59..29f7701c8 100644 --- a/assets/js/phoenix_live_view/view.js +++ b/assets/js/phoenix_live_view/view.js @@ -318,8 +318,12 @@ export default class View { this.formsForRecovery = this.getFormsForRecovery() } if(this.isMain() && window.history.state === null){ - // set initial history entry if this is the first page load - this.liveSocket.replaceRootHistory() + // set initial history entry if this is the first page load (no history) + Browser.pushState("replace", { + type: "patch", + id: this.id, + position: this.liveSocket.currentHistoryPosition + }) } if(liveview_version !== this.liveSocket.version()){ diff --git a/test/e2e/support/issues/issue_3529.ex b/test/e2e/support/issues/issue_3529.ex new file mode 100644 index 000000000..4a1c916e6 --- /dev/null +++ b/test/e2e/support/issues/issue_3529.ex @@ -0,0 +1,23 @@ +defmodule Phoenix.LiveViewTest.E2E.Issue3529Live do + # https://github.com/phoenixframework/phoenix_live_view/issues/3529 + + use Phoenix.LiveView + + alias Phoenix.LiveView.JS + + def mount(_params, _session, socket) do + {:ok, assign(socket, :mounted, DateTime.utc_now())} + end + + def handle_params(_params, _uri, socket) do + {:noreply, assign(socket, :next, :rand.uniform())} + end + + def render(assigns) do + ~H""" +

Mounted at {@mounted}

+ <.link navigate={"/issues/3529?param=#{@next}"}>Navigate + <.link patch={"/issues/3529?param=#{@next}"}>Patch + """ + end +end diff --git a/test/e2e/test_helper.exs b/test/e2e/test_helper.exs index 413a7e685..5f3418fa3 100644 --- a/test/e2e/test_helper.exs +++ b/test/e2e/test_helper.exs @@ -158,6 +158,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do live "/3448", Issue3448Live live "/3496/a", Issue3496.ALive live "/3496/b", Issue3496.BLive + live "/3529", Issue3529Live live "/3651", Issue3651Live end end diff --git a/test/e2e/tests/issues/3529.spec.js b/test/e2e/tests/issues/3529.spec.js new file mode 100644 index 000000000..f2af4cadc --- /dev/null +++ b/test/e2e/tests/issues/3529.spec.js @@ -0,0 +1,45 @@ +const {test, expect} = require("../../test-fixtures") +const {syncLV} = require("../../utils") + +const pageText = async (page) => await page.evaluate(() => document.querySelector("h1").innerText) + +// https://github.com/phoenixframework/phoenix_live_view/issues/3529 +// https://github.com/phoenixframework/phoenix_live_view/pull/3625 +test("forward and backward navigation is handled properly (replaceRootHistory)", async ({page}) => { + await page.goto("/issues/3529") + await syncLV(page) + + let text = await pageText(page) + await page.getByRole("link", {name: "Navigate"}).click() + await syncLV(page) + + // navigate remounts and changes the text + expect(await pageText(page)).not.toBe(text) + text = await pageText(page) + + await page.getByRole("link", {name: "Patch"}).click() + await syncLV(page) + // patch does not remount + expect(await pageText(page)).toBe(text) + + // now we go back (should be patch again) + await page.goBack() + await syncLV(page) + expect(await pageText(page)).toBe(text) + + // and then we back to the initial page and use back/forward + // this should be a navigate -> remount! + await page.goBack() + await syncLV(page) + expect(await pageText(page)).not.toBe(text) + + // navigate + await page.goForward() + await syncLV(page) + text = await pageText(page) + + // now back again (navigate) + await page.goBack() + await syncLV(page) + expect(await pageText(page)).not.toBe(text) +})