diff --git a/assets/js/phoenix_live_view/dom_patch.js b/assets/js/phoenix_live_view/dom_patch.js index c16ba09f8..09eb786d6 100644 --- a/assets/js/phoenix_live_view/dom_patch.js +++ b/assets/js/phoenix_live_view/dom_patch.js @@ -433,7 +433,7 @@ export default class DOMPatch { transitionPendingRemoves(){ let {pendingRemoves, liveSocket} = this if(pendingRemoves.length > 0){ - liveSocket.transitionRemoves(pendingRemoves, false, () => { + liveSocket.transitionRemoves(pendingRemoves, () => { pendingRemoves.forEach(el => { let child = DOM.firstPhxChild(el) if(child){ liveSocket.destroyViewByEl(child) } diff --git a/assets/js/phoenix_live_view/live_socket.js b/assets/js/phoenix_live_view/live_socket.js index b664307f3..136e2f630 100644 --- a/assets/js/phoenix_live_view/live_socket.js +++ b/assets/js/phoenix_live_view/live_socket.js @@ -393,22 +393,26 @@ export default class LiveSocket { } replaceMain(href, flash, callback = null, linkRef = this.setPendingLink(href)){ - let liveReferer = this.currentLocation.href + const liveReferer = this.currentLocation.href this.outgoingMainEl = this.outgoingMainEl || this.main.el - let removeEls = DOM.all(this.outgoingMainEl, `[${this.binding("remove")}]`) - let newMainEl = DOM.cloneNode(this.outgoingMainEl, "") + + const stickies = DOM.findPhxSticky(document) || [] + const removeEls = DOM.all(this.outgoingMainEl, `[${this.binding("remove")}]`) + .filter(el => !DOM.isChildOfAny(el, stickies)) + + const newMainEl = DOM.cloneNode(this.outgoingMainEl, "") this.main.showLoader(this.loaderTimeout) this.main.destroy() this.main = this.newRootView(newMainEl, flash, liveReferer) this.main.setRedirect(href) - this.transitionRemoves(removeEls, true) + this.transitionRemoves(removeEls) this.main.join((joinCount, onDone) => { if(joinCount === 1 && this.commitPendingLink(linkRef)){ this.requestDOMUpdate(() => { // remove phx-remove els right before we replace the main element removeEls.forEach(el => el.remove()) - DOM.findPhxSticky(document).forEach(el => newMainEl.appendChild(el)) + stickies.forEach(el => newMainEl.appendChild(el)) this.outgoingMainEl.replaceWith(newMainEl) this.outgoingMainEl = null callback && callback(linkRef) @@ -418,12 +422,8 @@ export default class LiveSocket { }) } - transitionRemoves(elements, skipSticky, callback){ + transitionRemoves(elements, callback){ let removeAttr = this.binding("remove") - if(skipSticky){ - const stickies = DOM.findPhxSticky(document) || [] - elements = elements.filter(el => !DOM.isChildOfAny(el, stickies)) - } let silenceEvents = (e) => { e.preventDefault() e.stopImmediatePropagation() @@ -800,7 +800,8 @@ export default class LiveSocket { } historyRedirect(e, href, linkState, flash, targetEl){ - if(targetEl && e.isTrusted && e.type !== "popstate"){ targetEl.classList.add("phx-click-loading") } + const clickLoading = targetEl && e.isTrusted && e.type !== "popstate" + if(clickLoading){ targetEl.classList.add("phx-click-loading") } if(!this.isConnected() || !this.main.isMain()){ return Browser.redirect(href, flash) } // convert to full href if only path prefix @@ -829,6 +830,9 @@ export default class LiveSocket { DOM.dispatchEvent(window, "phx:navigate", {detail: {href, patch: false, pop: false, direction: "forward"}}) this.registerNewLocation(window.location) } + // explicitly undo click-loading class + // (in case it originated in a sticky live view, otherwise it would be removed anyway) + if(clickLoading){ targetEl.classList.remove("phx-click-loading") } done() }) }) diff --git a/test/e2e/support/issues/issue_3656.ex b/test/e2e/support/issues/issue_3656.ex new file mode 100644 index 000000000..0004ef580 --- /dev/null +++ b/test/e2e/support/issues/issue_3656.ex @@ -0,0 +1,56 @@ +defmodule Phoenix.LiveViewTest.E2E.Issue3656Live do + # https://github.com/phoenixframework/phoenix_live_view/issues/3656 + + use Phoenix.LiveView + + alias Phoenix.LiveView.JS + + def mount(_params, _session, socket) do + {:ok, socket} + end + + def render(assigns) do + ~H""" + + + {live_render(@socket, Phoenix.LiveViewTest.E2E.Issue3656Live.Sticky, + id: "sticky", + sticky: true + )} + """ + end +end + +defmodule Phoenix.LiveViewTest.E2E.Issue3656Live.Sticky do + use Phoenix.LiveView + + def mount(:not_mounted_at_router, _session, socket) do + {:ok, socket, layout: false} + end + + def render(assigns) do + ~H""" + + """ + end +end diff --git a/test/e2e/support/issues/issue_3658.ex b/test/e2e/support/issues/issue_3658.ex new file mode 100644 index 000000000..bd1fbd098 --- /dev/null +++ b/test/e2e/support/issues/issue_3658.ex @@ -0,0 +1,38 @@ +defmodule Phoenix.LiveViewTest.E2E.Issue3658Live do + # https://github.com/phoenixframework/phoenix_live_view/issues/3658 + + use Phoenix.LiveView + + alias Phoenix.LiveView.JS + + def mount(_params, _session, socket) do + {:ok, socket} + end + + def render(assigns) do + ~H""" + <.link navigate="/issues/3658?navigated=true">Link 1 + + {live_render(@socket, Phoenix.LiveViewTest.E2E.Issue3658Live.Sticky, + id: "sticky", + sticky: true + )} + """ + end +end + +defmodule Phoenix.LiveViewTest.E2E.Issue3658Live.Sticky do + use Phoenix.LiveView + + def mount(:not_mounted_at_router, _session, socket) do + {:ok, socket, layout: false} + end + + def render(assigns) do + ~H""" +
+
Hi
+
+ """ + end +end diff --git a/test/e2e/test_helper.exs b/test/e2e/test_helper.exs index 9e8d1fb01..d00ade4ca 100644 --- a/test/e2e/test_helper.exs +++ b/test/e2e/test_helper.exs @@ -160,6 +160,8 @@ defmodule Phoenix.LiveViewTest.E2E.Router do live "/3496/b", Issue3496.BLive live "/3529", Issue3529Live live "/3651", Issue3651Live + live "/3656", Issue3656Live + live "/3658", Issue3658Live end end diff --git a/test/e2e/tests/issues/3656.spec.js b/test/e2e/tests/issues/3656.spec.js new file mode 100644 index 000000000..cf7fffbe4 --- /dev/null +++ b/test/e2e/tests/issues/3656.spec.js @@ -0,0 +1,21 @@ +const {test, expect} = require("../../test-fixtures") +const {syncLV, attributeMutations} = require("../../utils") + +// https://github.com/phoenixframework/phoenix_live_view/issues/3656 +test("phx-click-loading is removed from links in sticky LiveViews", async ({page}) => { + await page.goto("/issues/3656") + await syncLV(page) + + let changes = attributeMutations(page, "nav a") + + const link = page.getByRole("link", {name: "Link 1"}) + await link.click() + + await syncLV(page) + await expect(link).not.toHaveClass("phx-click-loading") + + expect(await changes()).toEqual(expect.arrayContaining([ + {attr: "class", oldValue: null, newValue: "phx-click-loading"}, + {attr: "class", oldValue: "phx-click-loading", newValue: ""}, + ])) +}) diff --git a/test/e2e/tests/issues/3658.spec.js b/test/e2e/tests/issues/3658.spec.js new file mode 100644 index 000000000..450d64fd4 --- /dev/null +++ b/test/e2e/tests/issues/3658.spec.js @@ -0,0 +1,15 @@ +const {test, expect} = require("../../test-fixtures") +const {syncLV} = require("../../utils") + +// https://github.com/phoenixframework/phoenix_live_view/issues/3658 +test("phx-remove elements inside sticky LiveViews are not removed when navigating", async ({page}) => { + await page.goto("/issues/3658") + await syncLV(page) + + await expect(page.locator("#foo")).toBeVisible() + await page.getByRole("link", {name: "Link 1"}).click() + + await syncLV(page) + // the bug would remove the element + await expect(page.locator("#foo")).toBeVisible() +})