From 2f1193f8761f3b346c2ce7b273d6d64e5d72b5cd Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 8 Nov 2023 09:09:36 +0100 Subject: [PATCH] Respect morphing and scroll preservation settings when handling form errors Turbo will render 422 responses to allow handling form errors. A common scenario in Rails is to render those setting the satus like: ``` render "edit", status: :unprocessable_entity ``` This change will consider such operations a "page refresh" and will also consider the scroll directive. --- src/core/drive/navigator.js | 4 +++- src/core/drive/page_view.js | 2 +- src/tests/fixtures/page_refresh.html | 7 +++++++ src/tests/functional/page_refresh_tests.js | 17 +++++++++++++++-- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js index 0d7259913..999d97817 100644 --- a/src/core/drive/navigator.js +++ b/src/core/drive/navigator.js @@ -99,7 +99,9 @@ export class Navigator { } else { await this.view.renderPage(snapshot, false, true, this.currentVisit) } - this.view.scrollToTop() + if(!snapshot.shouldPreserveScrollPosition) { + this.view.scrollToTop() + } this.view.clearSnapshotCache() } } diff --git a/src/core/drive/page_view.js b/src/core/drive/page_view.js index 6cf25dfd5..249b73efc 100644 --- a/src/core/drive/page_view.js +++ b/src/core/drive/page_view.js @@ -60,7 +60,7 @@ export class PageView extends View { } isPageRefresh(visit) { - return visit && this.lastRenderedLocation.href === visit.location.href + return !visit || this.lastRenderedLocation.href === visit.location.href } get snapshot() { diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html index d8d948a63..ec28092a4 100644 --- a/src/tests/fixtures/page_refresh.html +++ b/src/tests/fixtures/page_refresh.html @@ -49,6 +49,13 @@

Element with Stimulus controller

+
+
+ + +
+
+
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js index 272a34c09..70c798587 100644 --- a/src/tests/functional/page_refresh_tests.js +++ b/src/tests/functional/page_refresh_tests.js @@ -4,8 +4,9 @@ import { nextEventNamed, nextEventOnTarget, noNextEventOnTarget, - noNextEventNamed + noNextEventNamed, nextBody, hasSelector } from "../helpers/page" +import { assert } from "chai" test("renders a page refresh with morphing", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") @@ -53,7 +54,7 @@ test("don't refresh frames contained in [data-turbo-permanent] elements", async expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy() }) -test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => { +test("remote frames excluded from full page morphing", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") await page.evaluate(() => document.getElementById("remote-frame").setAttribute("data-modified", "true")) @@ -126,6 +127,18 @@ test("it preserves data-turbo-permanent elements that don't match when their ids await expect(page.locator("#preserve-me")).toHaveText("Preserve me, I have a family!") }) +test("renders unprocessable entity responses with morphing", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + + await page.click("#reject form.unprocessable_entity input[type=submit]") + await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) + await nextBody(page) + + const title = await page.locator("h1") + assert.equal(await title.textContent(), "Unprocessable Entity", "renders the response HTML") + assert.notOk(await hasSelector(page, "#frame form.reject"), "replaces entire page") +}) + async function assertPageScroll(page, top, left) { const [scrollTop, scrollLeft] = await page.evaluate(() => { return [