From c81830783eb498a3732a8248353146e262378532 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/autofocus.html | 9 +++++++++ src/tests/functional/autofocus_tests.js | 22 ++++++++++++++++++++++ src/tests/functional/page_refresh_tests.js | 2 +- 5 files changed, 45 insertions(+), 4 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/autofocus.html b/src/tests/fixtures/autofocus.html index a0d55fa0d..8d49539cb 100644 --- a/src/tests/fixtures/autofocus.html +++ b/src/tests/fixtures/autofocus.html @@ -5,6 +5,8 @@ Autofocus + +

Autofocus

@@ -22,5 +24,12 @@

Autofocus

#drives-frame link to frames/form.html + +
+ + + + +
diff --git a/src/tests/functional/autofocus_tests.js b/src/tests/functional/autofocus_tests.js index 726ef8307..a504d1bc9 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,24 @@ 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 }) => { + const input = await page.locator("#form input[autofocus]") + + const button = page.locator("#first-autofocus-element") + await button.click() + + await nextPageRefresh(page) + + await expect(button).toBeFocused() + await expect(input).not.toBeFocused() + + await page.evaluate(() => { + document.querySelector("#form").requestSubmit() + }) + + await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) + await nextPageRefresh(page) + + await expect(button).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]")