From 82864bd32e5e92fb16a6bdb44f54f5daab91a529 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 30 May 2024 13:05:39 +0200 Subject: [PATCH] Don't autofocus when rendering page refreshes with morphing Before this change, Turbo would always focus on the first element with `[autofocus]` when a page refresh with morphing happened (default behavior inherited from `PageRenderer`). With this change, it will never autofocus when a page refresh with morphing happens. This is meant to solve the problem where you are writing on a form, and it loses the focus because a broadcasted page refresh arrives. --- src/core/drive/morph_renderer.js | 4 ++++ src/core/renderer.js | 12 ++++++++--- src/tests/fixtures/page_refresh.html | 2 +- src/tests/functional/autofocus_tests.js | 23 ++++++++++++++++++++++ src/tests/functional/page_refresh_tests.js | 2 +- 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js index 2c5d14874..11142463e 100644 --- a/src/core/drive/morph_renderer.js +++ b/src/core/drive/morph_renderer.js @@ -11,6 +11,10 @@ export class MorphRenderer extends PageRenderer { return "morph" } + get shouldAutofocus() { + return false + } + // Private async #morphBody() { diff --git a/src/core/renderer.js b/src/core/renderer.js index 56e73983e..f0a02f581 100644 --- a/src/core/renderer.js +++ b/src/core/renderer.js @@ -16,6 +16,10 @@ export class Renderer { return true } + get shouldAutofocus() { + return true + } + get reloadReason() { return } @@ -40,9 +44,11 @@ export class Renderer { } focusFirstAutofocusableElement() { - const element = this.connectedSnapshot.firstAutofocusableElement - if (element) { - element.focus() + if (this.shouldAutofocus) { + const element = this.connectedSnapshot.firstAutofocusableElement + if (element) { + element.focus() + } } } diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html index ee2ff20b6..6142a0f06 100644 --- a/src/tests/fixtures/page_refresh.html +++ b/src/tests/fixtures/page_refresh.html @@ -113,7 +113,7 @@

Element with Stimulus controller

diff --git a/src/tests/functional/autofocus_tests.js b/src/tests/functional/autofocus_tests.js index 726ef8307..aedce9027 100644 --- a/src/tests/functional/autofocus_tests.js +++ b/src/tests/functional/autofocus_tests.js @@ -1,4 +1,5 @@ import { expect, test } from "@playwright/test" +import { nextEventNamed, nextPageRefresh } from "../helpers/page" test.beforeEach(async ({ page }) => { await page.goto("/src/tests/fixtures/autofocus.html") @@ -74,3 +75,25 @@ test("receiving a Turbo Stream message with an [autofocus] element when an eleme }) await expect(page.locator("#first-autofocus-element")).toBeFocused() }) + +test("don't focus on [autofocus] elements on page refreshes with morphing", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + + const input = await page.locator("input[autofocus]") + + const link = await page.locator("#refresh-after-navigation-link") + await link.focus() + + expect(link).toBeFocused() + expect(input).not.toBeFocused() + + await page.evaluate(() => { + document.querySelector("#form").requestSubmit() + }) + + await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) + await nextPageRefresh(page) + + expect(input).not.toBeFocused() + expect(link).toBeFocused() +}) diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js index c5c116c08..448b79285 100644 --- a/src/tests/functional/page_refresh_tests.js +++ b/src/tests/functional/page_refresh_tests.js @@ -109,7 +109,7 @@ test("renders a page refresh with morphing when the paths are the same but searc await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) }) -test("renders a page refresh with morphing when the GET form paths are the same but search params are diferent", async ({ page }) => { +test("renders a page refresh with morphing when the GET form paths are the same but search params are different", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") const input = page.locator("form[method=get] input[name=query]")