From 423743fddd4adb53efce9f7c899edb3154677d45 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 20 Feb 2024 14:38:25 -0500 Subject: [PATCH 01/20] Update JSDoc for `StreamElement` The JSDoc comment for the `StreamElement` is incorrect, and omits several actions. This commit sorts the list, replaces `"container"` with `"target"` (to mirror the attribute name), and mentions the missing actions. --- src/elements/stream_element.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/elements/stream_element.js b/src/elements/stream_element.js index b9bcf35f0..77f447594 100644 --- a/src/elements/stream_element.js +++ b/src/elements/stream_element.js @@ -6,20 +6,22 @@ import { nextRepaint } from "../util" /** * Renders updates to the page from a stream of messages. * - * Using the `action` attribute, this can be configured one of four ways: + * Using the `action` attribute, this can be configured one of eight ways: * - * - `append` - appends the result to the container - * - `prepend` - prepends the result to the container - * - `replace` - replaces the contents of the container - * - `remove` - removes the container - * - `before` - inserts the result before the target * - `after` - inserts the result after the target + * - `append` - appends the result to the target + * - `before` - inserts the result before the target + * - `prepend` - prepends the result to the target + * - `refresh` - initiates a page refresh + * - `remove` - removes the target + * - `replace` - replaces the outer HTML of the target + * - `update` - replaces the inner HTML of the target * * @customElement turbo-stream * @example * * * */ From 290d6cc16439874a2c4cee5987a18535d7d442dc Mon Sep 17 00:00:00 2001 From: Helen Date: Mon, 18 Mar 2024 10:18:04 -0700 Subject: [PATCH 02/20] fix bug in beforeNodeMorphed --- src/core/streams/actions/morph.js | 22 ++++++++++++---------- src/tests/unit/stream_element_tests.js | 13 +++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/core/streams/actions/morph.js b/src/core/streams/actions/morph.js index d177bfd01..42e655c95 100644 --- a/src/core/streams/actions/morph.js +++ b/src/core/streams/actions/morph.js @@ -26,17 +26,19 @@ function beforeNodeRemoved(node) { } function beforeNodeMorphed(target, newElement) { - if (target instanceof HTMLElement && !target.hasAttribute("data-turbo-permanent")) { - const event = dispatch("turbo:before-morph-element", { - cancelable: true, - target, - detail: { - newElement - } - }) - return !event.defaultPrevented + if (target instanceof HTMLElement) { + if (!target.hasAttribute("data-turbo-permanent")) { + const event = dispatch("turbo:before-morph-element", { + cancelable: true, + target, + detail: { + newElement + } + }) + return !event.defaultPrevented + } + return false } - return false } function beforeAttributeUpdated(attributeName, target, mutationType) { diff --git a/src/tests/unit/stream_element_tests.js b/src/tests/unit/stream_element_tests.js index 1e3b99f92..0d3e04b61 100644 --- a/src/tests/unit/stream_element_tests.js +++ b/src/tests/unit/stream_element_tests.js @@ -210,6 +210,19 @@ test("action=morph", async () => { assert.equal(subject.find("h1#hello")?.textContent, "Hello Turbo Morphed") }) +test("action=morph with text content change", async () => { + const templateElement = createTemplateElement(`
Hello Turbo Morphed
`) + const element = createStreamElement("morph", "hello", templateElement) + + assert.equal(subject.find("div#hello")?.textContent, "Hello Turbo") + + subject.append(element) + await nextAnimationFrame() + + assert.ok(subject.find("div#hello")) + assert.equal(subject.find("div#hello")?.textContent, "Hello Turbo Morphed") +}) + test("action=morph children-only", async () => { const templateElement = createTemplateElement(`

Hello Turbo Morphed

`) const element = createStreamElement("morph", "hello", templateElement) From 0ce534b3a244084a5a05a4ef344c6b2f571cecfa Mon Sep 17 00:00:00 2001 From: Helen Date: Mon, 18 Mar 2024 10:19:38 -0700 Subject: [PATCH 03/20] improve readability --- src/core/streams/actions/morph.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/core/streams/actions/morph.js b/src/core/streams/actions/morph.js index 42e655c95..e0813e12d 100644 --- a/src/core/streams/actions/morph.js +++ b/src/core/streams/actions/morph.js @@ -26,19 +26,20 @@ function beforeNodeRemoved(node) { } function beforeNodeMorphed(target, newElement) { - if (target instanceof HTMLElement) { - if (!target.hasAttribute("data-turbo-permanent")) { - const event = dispatch("turbo:before-morph-element", { - cancelable: true, - target, - detail: { - newElement - } - }) - return !event.defaultPrevented - } - return false + if (!(target instanceof HTMLElement)) { + return + } + if (!target.hasAttribute("data-turbo-permanent")) { + const event = dispatch("turbo:before-morph-element", { + cancelable: true, + target, + detail: { + newElement + } + }) + return !event.defaultPrevented } + return false } function beforeAttributeUpdated(attributeName, target, mutationType) { From 9c2bd18ff21d49bd9fc3d6df98822689d49c4b49 Mon Sep 17 00:00:00 2001 From: Helen Date: Mon, 25 Mar 2024 08:57:37 -0700 Subject: [PATCH 04/20] revert style change --- src/core/streams/actions/morph.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/core/streams/actions/morph.js b/src/core/streams/actions/morph.js index e0813e12d..42e655c95 100644 --- a/src/core/streams/actions/morph.js +++ b/src/core/streams/actions/morph.js @@ -26,20 +26,19 @@ function beforeNodeRemoved(node) { } function beforeNodeMorphed(target, newElement) { - if (!(target instanceof HTMLElement)) { - return - } - if (!target.hasAttribute("data-turbo-permanent")) { - const event = dispatch("turbo:before-morph-element", { - cancelable: true, - target, - detail: { - newElement - } - }) - return !event.defaultPrevented + if (target instanceof HTMLElement) { + if (!target.hasAttribute("data-turbo-permanent")) { + const event = dispatch("turbo:before-morph-element", { + cancelable: true, + target, + detail: { + newElement + } + }) + return !event.defaultPrevented + } + return false } - return false } function beforeAttributeUpdated(attributeName, target, mutationType) { From 513a0e01d6191d2f7c8e6dc643d2bf568db769f5 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Tue, 26 Mar 2024 23:17:08 -0400 Subject: [PATCH 05/20] Resolve Turbo Frame navigation bug The scenario --- Imagine a List-Details style page layout, with navigation links on the left and the contents of the page on the right: ```html Article #1 Some preview text Next Page

Details

``` The `#list` element is a `` to handle pagination, and the `#details` element is a `` as well. The `` elements within the `#list` frame drive the `#details` frame through their `[data-turbo-frame="details"]`. The `` element nests a third kind of `` that is specific to the resource being linked to. It asynchronously loads in preview content with a `[src]` attribute. The problem --- Clicking the `Article #1` text within the `` element drives the `#details` frame as expected. However, clicking the `Some preview text` within the `` element's nested `` element navigates the entire page. This is demonstrated in the following reproduction script: ```ruby require "bundler/inline" gemfile(true) do source "https://rubygems.org" git_source(:github) { |repo| "https://github.com/#{repo}.git" } gem "rails" gem "propshaft" gem "puma" gem "sqlite3" gem "turbo-rails" gem "capybara" gem "cuprite", "~> 0.9", require: "capybara/cuprite" end ENV["DATABASE_URL"] = "sqlite3::memory:" ENV["RAILS_ENV"] = "test" require "active_record/railtie" require "action_controller/railtie" require "action_view/railtie" require "action_cable/engine" require "rails/test_unit/railtie" class App < Rails::Application config.load_defaults Rails::VERSION::STRING.to_f config.root = __dir__ config.hosts << "example.org" config.eager_load = false config.session_store :cookie_store, key: "cookie_store_key" config.secret_key_base = "secret_key_base" config.consider_all_requests_local = true config.action_cable.cable = {"adapter" => "async"} config.turbo.draw_routes = false Rails.logger = config.logger = Logger.new($stdout) routes.append do root to: "application#index" end end Rails.application.initialize! ActiveRecord::Schema.define do create_table :messages, force: true do |t| t.text :body, null: false end end class Message < ActiveRecord::Base end class ApplicationController < ActionController::Base include Rails.application.routes.url_helpers class_attribute :template, default: DATA.read def index render inline: template, formats: :html end end class ApplicationSystemTestCase < ActionDispatch::SystemTestCase driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: { js_errors: true, headless: false } end Capybara.configure do |config| config.server = :puma, {Silent: true} config.default_normalize_ws = true end require "rails/test_help" class TurboSystemTest < ApplicationSystemTestCase test "reproduces bug" do visit root_path binding.irb click_link "Drive #details to ?key=1" end end __END__ <%= csrf_meta_tags %> <% 1.upto(5).each do |key| %> <%= link_to({key:}, data: {turbo_frame: "details"}) do %> Drive #details to <%= {key:}.to_query %> <% end %> <% end %> <%= params.fetch(:key, "0") %> ``` The solution --- When observing `click` events, utilize the `findLinkFromClickTarget` to find the nearest `` element to the `click` target so that **that** element's ancestors are used to determine which `` to target instead of risking the possibility of using one of its **descendants**. --- src/core/frames/link_interceptor.js | 14 +++++++++----- src/tests/fixtures/frames.html | 3 +++ src/tests/functional/frame_tests.js | 13 +++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/core/frames/link_interceptor.js b/src/core/frames/link_interceptor.js index 9b9249544..fb025db58 100644 --- a/src/core/frames/link_interceptor.js +++ b/src/core/frames/link_interceptor.js @@ -1,3 +1,5 @@ +import { findLinkFromClickTarget } from "../../util" + export class LinkInterceptor { constructor(delegate, element) { this.delegate = delegate @@ -17,7 +19,7 @@ export class LinkInterceptor { } clickBubbled = (event) => { - if (this.respondsToEventTarget(event.target)) { + if (this.clickEventIsSignificant(event)) { this.clickEvent = event } else { delete this.clickEvent @@ -25,7 +27,7 @@ export class LinkInterceptor { } linkClicked = (event) => { - if (this.clickEvent && this.respondsToEventTarget(event.target) && event.target instanceof Element) { + if (this.clickEvent && this.clickEventIsSignificant(event)) { if (this.delegate.shouldInterceptLinkClick(event.target, event.detail.url, event.detail.originalEvent)) { this.clickEvent.preventDefault() event.preventDefault() @@ -39,8 +41,10 @@ export class LinkInterceptor { delete this.clickEvent } - respondsToEventTarget(target) { - const element = target instanceof Element ? target : target instanceof Node ? target.parentElement : null - return element && element.closest("turbo-frame, html") == this.element + clickEventIsSignificant(event) { + const target = event.composed ? event.target?.parentElement : event.target + const element = findLinkFromClickTarget(target) || target + + return element instanceof Element && element.closest("turbo-frame, html") == this.element } } diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html index 24c3272b9..0513e911a 100644 --- a/src/tests/fixtures/frames.html +++ b/src/tests/fixtures/frames.html @@ -37,6 +37,9 @@

Frames: #frame

Navigate #frame to /frames/form.html + + Has a turbo-frame child + Navigate #frame from outside with a[data-turbo-action="advance"]
diff --git a/src/tests/functional/frame_tests.js b/src/tests/functional/frame_tests.js index f1925908b..3b0770c39 100644 --- a/src/tests/functional/frame_tests.js +++ b/src/tests/functional/frame_tests.js @@ -454,6 +454,19 @@ test("'turbo:frame-render' is triggered after frame has finished rendering", asy assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/part.html") }) +test("navigating a frame from an outer link with a turbo-frame child fires events", async ({ page }) => { + await page.click("#outside-frame-link-with-frame-child") + + await nextEventOnTarget(page, "frame", "turbo:before-fetch-request") + await nextEventOnTarget(page, "frame", "turbo:before-fetch-response") + const { fetchResponse } = await nextEventOnTarget(page, "frame", "turbo:frame-render") + expect(fetchResponse.response.url).toContain("/src/tests/fixtures/frames/form.html") + + await nextEventOnTarget(page, "frame", "turbo:frame-load") + + expect(await readEventLogs(page), "no more events").toHaveLength(0) +}) + test("navigating a frame from an outer form fires events", async ({ page }) => { await page.click("#outside-frame-form") From 8884c1e7ba4f5ec1e9c9f3af4e9b8a2a7f5034a7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 28 Mar 2024 20:12:26 +0000 Subject: [PATCH 06/20] Bump express from 4.18.2 to 4.19.2 Bumps [express](https://github.com/expressjs/express) from 4.18.2 to 4.19.2. - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/master/History.md) - [Commits](https://github.com/expressjs/express/compare/4.18.2...4.19.2) --- updated-dependencies: - dependency-name: express dependency-type: direct:development ... Signed-off-by: dependabot[bot] --- yarn.lock | 67 ++++++++++++++++++++++++------------------------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/yarn.lock b/yarn.lock index 5b147bcc4..54d90f805 100644 --- a/yarn.lock +++ b/yarn.lock @@ -796,13 +796,13 @@ bl@^4.0.3: inherits "^2.0.4" readable-stream "^3.4.0" -body-parser@1.20.1, body-parser@^1.20.1: - version "1.20.1" - resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.1.tgz#b1812a8912c195cd371a3ee5e66faa2338a5c668" - integrity sha512-jWi7abTbYwajOytWCQc37VulmWiRae5RyTpaCyDcS5/lMdtwSz5lOpDE67srw/HYe35f1z3fDQw+3txg7gNtWw== +body-parser@1.20.2, body-parser@^1.20.1: + version "1.20.2" + resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd" + integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA== dependencies: bytes "3.1.2" - content-type "~1.0.4" + content-type "~1.0.5" debug "2.6.9" depd "2.0.0" destroy "1.2.0" @@ -810,7 +810,7 @@ body-parser@1.20.1, body-parser@^1.20.1: iconv-lite "0.4.24" on-finished "2.4.1" qs "6.11.0" - raw-body "2.5.1" + raw-body "2.5.2" type-is "~1.6.18" unpipe "1.0.0" @@ -1061,16 +1061,11 @@ content-disposition@0.5.4, content-disposition@~0.5.2: dependencies: safe-buffer "5.2.1" -content-type@^1.0.4: +content-type@^1.0.4, content-type@~1.0.4, content-type@~1.0.5: version "1.0.5" resolved "https://registry.yarnpkg.com/content-type/-/content-type-1.0.5.tgz#8b773162656d1d1086784c8f23a54ce6d73d7918" integrity sha512-nTjqfcBFEipKdXCv4YDQWCfmcLZKm81ldF0pAopTvyrFGVbcR6P/VAAd5G7N+0tTr8QqiU0tFadD6FK4NtJwOA== -content-type@~1.0.4: - version "1.0.4" - resolved "https://registry.yarnpkg.com/content-type/-/content-type-1.0.4.tgz#e138cc75e040c727b1966fe5e5f8c9aee256fe3b" - integrity sha512-hIP3EEPs8tB9AT1L+NUqtwOAps4mk2Zob89MWXMHjHWg9milF/j4osnnQLXBCBFBk/tvIG/tUc9mOUJiPBhPXA== - convert-source-map@^1.6.0: version "1.9.0" resolved "https://registry.yarnpkg.com/convert-source-map/-/convert-source-map-1.9.0.tgz#7faae62353fb4213366d0ca98358d22e8368b05f" @@ -1088,10 +1083,10 @@ cookie-signature@1.0.6: resolved "https://registry.yarnpkg.com/cookie-signature/-/cookie-signature-1.0.6.tgz#e303a882b342cc3ee8ca513a79999734dab3ae2c" integrity sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ== -cookie@0.5.0: - version "0.5.0" - resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.5.0.tgz#d1f5d71adec6558c58f389987c366aa47e994f8b" - integrity sha512-YZ3GUyn/o8gfKJlnlX7g7xq4gyO6OSuhGPKaaGssGB2qgDUS0gPgtTvoyZLTt9Ab6dC4hfc9dV5arkvc/OCmrw== +cookie@0.6.0: + version "0.6.0" + resolved "https://registry.yarnpkg.com/cookie/-/cookie-0.6.0.tgz#2798b04b071b0ecbff0dbb62a505a8efa4e19051" + integrity sha512-U71cyTamuh1CRNCfpGY6to28lxvNwPG4Guz/EVjgf3Jmzv0vlDp1atT9eS5dDjMYHucpHbWns6Lwf3BKz6svdw== cookies@~0.8.0: version "0.8.0" @@ -1134,7 +1129,7 @@ debug@2.6.9, debug@^2.6.9: dependencies: ms "2.0.0" -debug@4, debug@4.3.4, debug@^4.3.2: +debug@4, debug@4.3.4, debug@^4.1.1, debug@^4.3.2: version "4.3.4" resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.4.tgz#1319f6579357f2338d3337d2cdd4914bb5dcc865" integrity sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ== @@ -1148,13 +1143,6 @@ debug@^3.1.0, debug@^3.2.7: dependencies: ms "^2.1.1" -debug@^4.1.1: - version "4.3.1" - resolved "https://registry.npmjs.org/debug/-/debug-4.3.1.tgz" - integrity sha512-doEwdvm4PCeK4K3RQN2ZC2BYUBaxwLARCqZmMjtF8a51J2Rb0xpVloFRnCODwqjpwnAoao4pelN8l3RJdv3gRQ== - dependencies: - ms "2.1.2" - deep-eql@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/deep-eql/-/deep-eql-3.0.1.tgz#dfc9404400ad1c8fe023e7da1df1c147c4b444df" @@ -1528,16 +1516,16 @@ etag@^1.8.1, etag@~1.8.1: integrity sha512-aIL5Fx7mawVa300al2BnEE4iNvo1qETxLrPI/o05L7z6go7fCw1J6EQmbK4FmJ2AS7kgVF/KEZWufBfdClMcPg== express@^4.18.2: - version "4.18.2" - resolved "https://registry.yarnpkg.com/express/-/express-4.18.2.tgz#3fabe08296e930c796c19e3c516979386ba9fd59" - integrity sha512-5/PsL6iGPdfQ/lKM1UuielYgv3BUoJfz1aUwU9vHZ+J7gyvwdQXFEBIEIaxeGf0GIcreATNyBExtalisDbuMqQ== + version "4.19.2" + resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" + integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== dependencies: accepts "~1.3.8" array-flatten "1.1.1" - body-parser "1.20.1" + body-parser "1.20.2" content-disposition "0.5.4" content-type "~1.0.4" - cookie "0.5.0" + cookie "0.6.0" cookie-signature "1.0.6" debug "2.6.9" depd "2.0.0" @@ -2406,20 +2394,13 @@ object-inspect@^1.9.0: resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.12.3.tgz#ba62dffd67ee256c8c086dfae69e016cd1f198b9" integrity sha512-geUvdk7c+eizMNUDkRpW1wJwgfOiOeHbxBR/hLXK1aT6zmVSO0jsQcs7fj6MGw89jC/cjGfLcNOrtMYtGqm81g== -on-finished@2.4.1: +on-finished@2.4.1, on-finished@^2.3.0: version "2.4.1" resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.4.1.tgz#58c8c44116e54845ad57f14ab10b03533184ac3f" integrity sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg== dependencies: ee-first "1.1.1" -on-finished@^2.3.0: - version "2.3.0" - resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.3.0.tgz#20f1336481b083cd75337992a16971aa2d906947" - integrity sha512-ikqdkGAAyf/X/gPhXGvfgAytDZtDbr+bkNUJ0N9h5MI/dmdgCs3l6hoHrcUv41sRKew3jIwrp4qQDXiK99Utww== - dependencies: - ee-first "1.1.1" - once@^1.3.0, once@^1.3.1, once@^1.4.0: version "1.4.0" resolved "https://registry.npmjs.org/once/-/once-1.4.0.tgz" @@ -2660,7 +2641,17 @@ range-parser@~1.2.1: resolved "https://registry.yarnpkg.com/range-parser/-/range-parser-1.2.1.tgz#3cf37023d199e1c24d1a55b84800c2f3e6468031" integrity sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg== -raw-body@2.5.1, raw-body@^2.3.3: +raw-body@2.5.2: + version "2.5.2" + resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.2.tgz#99febd83b90e08975087e8f1f9419a149366b68a" + integrity sha512-8zGqypfENjCIqGhgXToC8aB2r7YrBX+AQAfIPs/Mlk+BtPTztOvTS01NRW/3Eh60J+a48lt8qsCzirQ6loCVfA== + dependencies: + bytes "3.1.2" + http-errors "2.0.0" + iconv-lite "0.4.24" + unpipe "1.0.0" + +raw-body@^2.3.3: version "2.5.1" resolved "https://registry.yarnpkg.com/raw-body/-/raw-body-2.5.1.tgz#fe1b1628b181b700215e5fd42389f98b71392857" integrity sha512-qqJBtEyVgS0ZmPGdCFPWJ3FreoqvG4MVQln/kCgF7Olq95IbOp0/BWyMwbdtn4VTvkM8Y7khCQ2Xgk/tcrCXig== From 4d8af5331ebe859a773aebff6ce41acf317af319 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 29 Feb 2024 21:44:53 -0500 Subject: [PATCH 07/20] Prevent Refresh from interrupting ongoing Visit Closes [#1209] Defers the `Session.refresh` calls while handling `` elements if there is a `Visit` that's already initiated and being handled by the `Navigator`. [#1209]: https://github.com/hotwired/turbo/issues/1209 --- src/core/drive/navigator.js | 1 + src/core/session.js | 2 +- src/tests/fixtures/page_refresh.html | 1 + src/tests/fixtures/test.js | 4 ++++ src/tests/functional/page_refresh_tests.js | 27 ++++++++++++++++++---- src/tests/helpers/page.js | 10 ++++++-- 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js index 8d8d3e0be..8210c7471 100644 --- a/src/core/drive/navigator.js +++ b/src/core/drive/navigator.js @@ -125,6 +125,7 @@ export class Navigator { visitCompleted(visit) { this.delegate.visitCompleted(visit) + delete this.currentVisit } locationWithActionIsSamePage(location, action) { diff --git a/src/core/session.js b/src/core/session.js index cdb978348..1047d4463 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -109,7 +109,7 @@ export class Session { refresh(url, requestId) { const isRecentRequest = requestId && this.recentRequests.has(requestId) - if (!isRecentRequest) { + if (!isRecentRequest && !this.navigator.currentVisit) { this.visit(url, { action: "replace", shouldCacheSnapshot: false }) } } diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html index c0586677f..df5338b43 100644 --- a/src/tests/fixtures/page_refresh.html +++ b/src/tests/fixtures/page_refresh.html @@ -81,6 +81,7 @@

Page to be refreshed

Reload + Navigate with delayed response

Frame to be morphed

diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js index 7ba0f39e7..0dd66d48b 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -91,6 +91,10 @@ "turbo:reload" ]) +window.visitLogs = [] + +addEventListener("turbo:visit", ({ detail }) => window.visitLogs.push(detail)) + customElements.define( "custom-link-element", class extends HTMLElement { diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js index c5c116c08..84d765cb5 100644 --- a/src/tests/functional/page_refresh_tests.js +++ b/src/tests/functional/page_refresh_tests.js @@ -8,7 +8,8 @@ import { nextEventOnTarget, noNextEventOnTarget, noNextEventNamed, - getSearchParam + getSearchParam, + refreshWithStream } from "../helpers/page" test("renders a page refresh with morphing", async ({ page }) => { @@ -26,16 +27,32 @@ test("async page refresh with turbo-stream", async ({ page }) => { await page.evaluate(() => document.querySelector("#title").innerText = "Updated") await expect(page.locator("#title")).toHaveText("Updated") - - await page.evaluate(() => { - document.body.insertAdjacentHTML("beforeend", ``) - }) + await refreshWithStream(page) await expect(page.locator("#title")).not.toHaveText("Updated") await expect(page.locator("#title")).toHaveText("Page to be refreshed") expect(await noNextEventNamed(page, "turbo:before-cache")).toBeTruthy() }) +test("async page refresh with turbo-stream sequentially initiate Visits", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + await refreshWithStream(page) + await nextEventNamed(page, "turbo:morph") + await nextEventNamed(page, "turbo:load") + + await refreshWithStream(page) + await nextEventNamed(page, "turbo:morph") + await nextEventNamed(page, "turbo:load") +}) + +test("async page refresh with turbo-stream does not interrupt an initiated Visit", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + await page.click("#delayed_link") + await refreshWithStream(page) + + await expect(page.locator("h1")).toHaveText("One") +}) + test("dispatches a turbo:before-morph-element and turbo:morph-element event for each morphed element", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") await page.fill("#form-text", "Morph me") diff --git a/src/tests/helpers/page.js b/src/tests/helpers/page.js index 760b6631f..34069bed7 100644 --- a/src/tests/helpers/page.js +++ b/src/tests/helpers/page.js @@ -226,6 +226,10 @@ export function readMutationLogs(page, length) { return readArray(page, "mutationLogs", length) } +export function refreshWithStream(page) { + return page.evaluate(() => document.body.insertAdjacentHTML("beforeend", ``)) +} + export function search(url) { const { search } = new URL(url) @@ -284,8 +288,10 @@ export function textContent(page, html) { export function visitAction(page) { return page.evaluate(() => { try { - return window.Turbo.navigator.currentVisit.action - } catch (error) { + const lastVisit = window.visitLogs[window.visitLogs.length - 1] + + return lastVisit.action + } catch { return "load" } }) From 38821d70e851f32d88ab36c0f3662c73425e1ff3 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 24 Feb 2024 10:12:27 -0800 Subject: [PATCH 08/20] Add `turbo:before-prefetch` test coverage Expand the link prefetching functional test coverage to exercise prefetching in relation to the events dispatched during the lifecycle. For example, include coverage for: * `turbo:before-prefetch` being dispatched * `turbo:before-prefetch` being canceled * `turbo:before-fetch-request` dispatched only once during a successful prefetch --- src/tests/fixtures/hover_to_prefetch.html | 1 + src/tests/fixtures/test.js | 1 + .../link_prefetch_observer_tests.js | 38 +++++++++++++------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index d47be49db..bd0f6f944 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -4,6 +4,7 @@ Hover to Prefetch + diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js index 7ba0f39e7..39e2347eb 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -73,6 +73,7 @@ "turbo:before-visit", "turbo:load", "turbo:render", + "turbo:before-prefetch", "turbo:before-fetch-request", "turbo:submit-start", "turbo:submit-end", diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 735efb7fb..b3d362880 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -1,6 +1,6 @@ -import { test } from "@playwright/test" +import { expect, test } from "@playwright/test" import { assert } from "chai" -import { nextBeat, sleep } from "../helpers/page" +import { nextBeat, nextEventOnTarget, noNextEventNamed, noNextEventOnTarget, sleep } from "../helpers/page" import fs from "fs" import path from "path" @@ -17,7 +17,22 @@ test.afterEach(() => { test("it prefetches the page", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + + const link = page.locator("#anchor_for_prefetch") + + await link.hover() + await nextEventOnTarget(page, "anchor_for_prefetch", "turbo:before-prefetch") + const { url, fetchOptions } = await nextEventOnTarget(page, "anchor_for_prefetch", "turbo:before-fetch-request") + + expect(url).toEqual(await link.evaluate(a => a.href)) + expect(fetchOptions.headers["X-Sec-Purpose"]).toEqual("prefetch") + + await link.hover() + await noNextEventOnTarget(page, "anchor_for_prefetch", "turbo:before-fetch-request") + await link.click() + await noNextEventOnTarget(page, "anchor_for_prefetch", "turbo:before-fetch-request") + + await expect(page.locator("body")).toHaveText("Prefetched Page Content") }) test("it doesn't follow the link", async ({ page }) => { @@ -65,17 +80,16 @@ test("it doesn't prefetch the page when link has data-turbo=false", async ({ pag test("allows to cancel prefetch requests with custom logic", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + const link = page.locator("#anchor_for_prefetch") + await link.evaluate(a => a.addEventListener("turbo:before-prefetch", event => event.preventDefault())) - await page.evaluate(() => { - document.body.addEventListener("turbo:before-prefetch", (event) => { - if (event.target.hasAttribute("data-remote")) { - event.preventDefault() - } - }) - }) + await page.pause() + await link.hover() + await nextEventOnTarget(page, "anchor_for_prefetch", "turbo:before-prefetch") + await noNextEventNamed(page, "turbo:before-fetch-request") + await link.click() - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + await expect(page.locator("body")).toHaveText("Prefetched Page Content") }) test("it doesn't prefetch UJS links", async ({ page }) => { From 5bc21c09ff90509081a094566fae33b1ed33620c Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 30 Mar 2024 20:59:54 -0400 Subject: [PATCH 09/20] Ignore links and forms that target `"_blank"` Closes [#1171][] Forms that target [_blank][form-target-blank] along with anchors that target [_blank][] navigate new tabs, and are therefore incompatible with Turbo's long-lived session. This commit ensures that both `` and `` submissions that target `_blank` are ignored. [#1171]: https://github.com/hotwired/turbo/issues/1171 [form-target-blank]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form#target [anchor-target-blank]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target --- src/observers/form_submit_observer.js | 14 ++++---------- src/observers/link_click_observer.js | 2 +- src/tests/fixtures/navigation.html | 14 ++++++++++---- src/tests/functional/navigation_tests.js | 14 +++++++++++++- src/util.js | 14 +++++++++----- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/observers/form_submit_observer.js b/src/observers/form_submit_observer.js index 1158d59b9..204e4665a 100644 --- a/src/observers/form_submit_observer.js +++ b/src/observers/form_submit_observer.js @@ -1,3 +1,5 @@ +import { doesNotTargetIFrame } from "../util" + export class FormSubmitObserver { started = false @@ -51,15 +53,7 @@ function submissionDoesNotDismissDialog(form, submitter) { } function submissionDoesNotTargetIFrame(form, submitter) { - if (submitter?.hasAttribute("formtarget") || form.hasAttribute("target")) { - const target = submitter?.getAttribute("formtarget") || form.target - - for (const element of document.getElementsByName(target)) { - if (element instanceof HTMLIFrameElement) return false - } + const target = submitter?.getAttribute("formtarget") || form.getAttribute("target") - return true - } else { - return true - } + return doesNotTargetIFrame(target) } diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js index 24c6aa235..c1e6abfb7 100644 --- a/src/observers/link_click_observer.js +++ b/src/observers/link_click_observer.js @@ -31,7 +31,7 @@ export class LinkClickObserver { if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) { const target = (event.composedPath && event.composedPath()[0]) || event.target const link = findLinkFromClickTarget(target) - if (link && doesNotTargetIFrame(link)) { + if (link && doesNotTargetIFrame(link.target)) { const location = getLocationForLink(link) if (this.delegate.willFollowLinkToLocation(link, location, event)) { event.preventDefault() diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html index 7b698a613..cd43d2b06 100644 --- a/src/tests/fixtures/navigation.html +++ b/src/tests/fixtures/navigation.html @@ -94,24 +94,30 @@

Navigation

Delayed failure link

Targets iframe[name="iframe"]

Targets iframe[name=""]

+

+ + +

+

- +

- +

- + +

- +

Redirect to cache_observer.html

diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js index e1aa7e757..eebef546c 100644 --- a/src/tests/functional/navigation_tests.js +++ b/src/tests/functional/navigation_tests.js @@ -1,4 +1,4 @@ -import { test } from "@playwright/test" +import { expect, test } from "@playwright/test" import { assert } from "chai" import { clickWithoutScrolling, @@ -473,6 +473,12 @@ test("ignores links with a [target] attribute that targets an iframe with [name= assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html") }) +test("ignores forms with a [target=_blank] attribute", async ({ page }) => { + const [popup] = await Promise.all([page.waitForEvent("popup"), page.click("#form-target-blank button")]) + + expect(pathname(popup.url())).toContain("/src/tests/fixtures/one.html") +}) + test("ignores forms with a [target] attribute that targets an iframe with a matching [name]", async ({ page }) => { await page.click("#form-target-iframe button") await nextBeat() @@ -482,6 +488,12 @@ test("ignores forms with a [target] attribute that targets an iframe with a matc assert.equal(await pathnameForIFrame(page, "iframe"), "/src/tests/fixtures/one.html") }) +test("ignores forms with a button[formtarget=_blank] attribute", async ({ page }) => { + const [popup] = await Promise.all([page.waitForEvent("popup"), page.click("#button-formtarget-blank")]) + + expect(pathname(popup.url())).toContain("/src/tests/fixtures/one.html") +}) + test("ignores forms with a button[formtarget] attribute that targets an iframe with [name='']", async ({ page }) => { diff --git a/src/util.js b/src/util.js index dcb15c31b..57c48ea6a 100644 --- a/src/util.js +++ b/src/util.js @@ -218,14 +218,18 @@ export async function around(callback, reader) { return [before, after] } -export function doesNotTargetIFrame(anchor) { - if (anchor.hasAttribute("target")) { - for (const element of document.getElementsByName(anchor.target)) { +export function doesNotTargetIFrame(name) { + if (name === "_blank") { + return false + } else if (name) { + for (const element of document.getElementsByName(name)) { if (element instanceof HTMLIFrameElement) return false } - } - return true + return true + } else { + return true + } } export function findLinkFromClickTarget(target) { From cd16067186355d230b6694106056f55088b84fdc Mon Sep 17 00:00:00 2001 From: Stephen Mitchell Date: Mon, 20 May 2024 05:08:47 -0400 Subject: [PATCH 10/20] Check new snapshot (instead of previous) for refresh-method (#1123) * Update page_view.js * Update page_view.js * Adds 422 page for morphing test * Remove current page check * Update page_view.js * Update autofocus_tests.js * Update autofocus_tests.js --- src/core/drive/page_view.js | 2 +- src/tests/fixtures/422_morph.html | 16 ++++++++++++++++ src/tests/fixtures/page_refresh.html | 2 +- src/tests/functional/autofocus_tests.js | 2 ++ src/tests/server.mjs | 7 +++++++ 5 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 src/tests/fixtures/422_morph.html diff --git a/src/core/drive/page_view.js b/src/core/drive/page_view.js index 1583f25a0..6a40a94d9 100644 --- a/src/core/drive/page_view.js +++ b/src/core/drive/page_view.js @@ -16,7 +16,7 @@ export class PageView extends View { } renderPage(snapshot, isPreview = false, willRender = true, visit) { - const shouldMorphPage = this.isPageRefresh(visit) && this.snapshot.shouldMorphPage + const shouldMorphPage = this.isPageRefresh(visit) && snapshot.shouldMorphPage const rendererClass = shouldMorphPage ? MorphRenderer : PageRenderer const renderer = new rendererClass(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender) diff --git a/src/tests/fixtures/422_morph.html b/src/tests/fixtures/422_morph.html new file mode 100644 index 000000000..d87ec4161 --- /dev/null +++ b/src/tests/fixtures/422_morph.html @@ -0,0 +1,16 @@ + + + + + + Unprocessable Entity + + + +

Unprocessable Entity

+ + +

Frame: Unprocessable Entity

+
+ + diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html index c0586677f..ee2ff20b6 100644 --- a/src/tests/fixtures/page_refresh.html +++ b/src/tests/fixtures/page_refresh.html @@ -131,7 +131,7 @@

Element with Stimulus controller

-
+
diff --git a/src/tests/functional/autofocus_tests.js b/src/tests/functional/autofocus_tests.js index 9cc49afe3..726ef8307 100644 --- a/src/tests/functional/autofocus_tests.js +++ b/src/tests/functional/autofocus_tests.js @@ -59,7 +59,9 @@ test("autofocus from a Turbo Stream message does not leak a placeholder [id]", a `) }) + await expect(page.locator("#container-from-stream input")).toBeFocused() + }) test("receiving a Turbo Stream message with an [autofocus] element when an element within the document has focus", async ({ page }) => { diff --git a/src/tests/server.mjs b/src/tests/server.mjs index 10eab3abb..76cdaf464 100644 --- a/src/tests/server.mjs +++ b/src/tests/server.mjs @@ -61,6 +61,13 @@ router.post("/reject/tall", (request, response) => { response.status(parseInt(status || "422", 10)).sendFile(fixture) }) +router.post("/reject/morph", (request, response) => { + const { status } = request.body + const fixture = path.join(__dirname, `../../src/tests/fixtures/422_morph.html`) + + response.status(parseInt(status || "422", 10)).sendFile(fixture) +}) + router.post("/reject", (request, response) => { const { status } = request.body const fixture = path.join(__dirname, `../../src/tests/fixtures/${status}.html`) From 8c706bb894e05ea14448f0098aa32a7446836332 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Mon, 20 May 2024 11:11:02 +0200 Subject: [PATCH 11/20] Fix test to verify no network request on prefetched link click (#1257) * Fix test not making a network request after prefetch click The test was supposed to assert that a network request was not made when clicking on a link that had been prefetched by hovering over it. However, the test was not actually making the click, just trying to hover over an already hovered link, so this wasn't really testing what it was supposed to. * assertRequestMade and assertRequestNotMade take functions as arguments These functions are called to perform the action that should trigger the network request. This allows us to test different scenarios without repeating the same code. It also allows us to pass a callback that will be called with the request object when the request is made. This is useful for testing the request object itself. --- .../link_prefetch_observer_tests.js | 99 +++++++++---------- 1 file changed, 48 insertions(+), 51 deletions(-) diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 735efb7fb..361c8b020 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -195,35 +195,28 @@ test("doesn't include a turbo-frame header when the link is inside a turbo frame test("it prefetches links with a delay", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - let requestMade = false - page.on("request", async (request) => (requestMade = true)) - - await page.hover("#anchor_for_prefetch") - await sleep(75) - - assertRequestNotMade(requestMade) - - await sleep(100) + await assertRequestNotMade(page, async () => { + await page.hover("#anchor_for_prefetch") + await sleep(75) + }) - assertRequestMade(requestMade) + await assertRequestMade(page, async () => { + await sleep(100) + }) }) test("it cancels the prefetch request if the link is no longer hovered", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - let requestMade = false - page.on("request", async (request) => (requestMade = true)) - - await page.hover("#anchor_for_prefetch") - await sleep(75) - - assertRequestNotMade(requestMade) - - await page.mouse.move(0, 0) - - await sleep(100) + await assertRequestNotMade(page, async () => { + await page.hover("#anchor_for_prefetch") + await sleep(75) + }) - assertRequestNotMade(requestMade) + await assertRequestNotMade(page, async () => { + await page.mouse.move(0, 0) + await sleep(100) + }) }) test("it resets the cache when a link is hovered", async ({ page }) => { @@ -246,11 +239,8 @@ test("it resets the cache when a link is hovered", async ({ page }) => { test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - await hoverSelector({ page, selector: "#anchor_for_prefetch" }) - - await sleep(100) - - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + await assertRequestNotMadeOnClick({ page, selector: "#anchor_for_prefetch" }) }) test("it follows the link using the cached response when clicking on a link that has been prefetched", async ({ @@ -264,44 +254,51 @@ test("it follows the link using the cached response when clicking on a link that }) const assertPrefetchedOnHover = async ({ page, selector, callback }) => { - let requestMade = false + await assertRequestMade(page, async () => { + await hoverSelector({ page, selector }) + + await sleep(100) + }, callback) +} - page.on("request", (request) => { - requestMade = request +const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { + await assertRequestNotMade(page, async () => { + await hoverSelector({ page, selector }) + + await sleep(100) + }, callback) +} + +const assertRequestNotMadeOnClick = async ({ page, selector }) => { + await assertRequestNotMade(page, async () => { + await clickSelector({ page, selector }) }) +} - await hoverSelector({ page, selector }) +const assertRequestMade = async (page, action, callback) => { + let requestMade = false + page.on("request", async (request) => requestMade = request) - await sleep(100) + await action() + + assert.equal(!!requestMade, true, "Network request wasn't made when it should have been.") if (callback) { await callback(requestMade) } - - assertRequestMade(!!requestMade) } -const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { +const assertRequestNotMade = async (page, action, callback) => { let requestMade = false + page.on("request", async (request) => requestMade = request) - page.on("request", (request) => { - callback && callback(request) - requestMade = true - }) - - await hoverSelector({ page, selector }) - - await sleep(100) - - assert.equal(requestMade, false, "Network request was made when it should not have been.") -} + await action() -const assertRequestMade = (requestMade) => { - assert.equal(requestMade, true, "Network request wasn't made when it should have been.") -} + assert.equal(!!requestMade, false, "Network request was made when it should not have been.") -const assertRequestNotMade = (requestMade) => { - assert.equal(requestMade, false, "Network request was made when it should not have been.") + if (callback) { + await callback(requestMade) + } } const goTo = async ({ page, path }) => { From 89be8e4c206ea877183b7ac4a89f898363a14f0f Mon Sep 17 00:00:00 2001 From: Tom Pietrosanti Date: Tue, 21 May 2024 10:07:27 -0400 Subject: [PATCH 12/20] style: align prettier trailing comma with eslint (#1260) --- .prettierrc.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.prettierrc.json b/.prettierrc.json index db5bf647b..ec8e5e030 100644 --- a/.prettierrc.json +++ b/.prettierrc.json @@ -1,5 +1,6 @@ { "singleQuote": false, "printWidth": 120, - "semi": false + "semi": false, + "trailingComma" : "none" } From c81830783eb498a3732a8248353146e262378532 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 30 May 2024 13:05:39 +0200 Subject: [PATCH 13/20] 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]") From 6843000e55d603f344a299b548b89cf5d04ccf59 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 19 Jun 2024 05:18:45 +0000 Subject: [PATCH 14/20] Bump braces from 3.0.2 to 3.0.3 Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](https://github.com/micromatch/braces/compare/3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/yarn.lock b/yarn.lock index 5b147bcc4..cb0dda3b0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -823,11 +823,11 @@ brace-expansion@^1.1.7: concat-map "0.0.1" braces@^3.0.2, braces@~3.0.2: - version "3.0.2" - resolved "https://registry.yarnpkg.com/braces/-/braces-3.0.2.tgz#3454e1a462ee8d599e236df336cd9ea4f8afe107" - integrity sha512-b8um+L1RzM3WDSzvhm6gIz1yfTbBt6YTlcEKAvsmqCZZFw46z626lVj9j1yEPW33H5H+lBQpZMP1k8l+78Ha0A== + version "3.0.3" + resolved "https://registry.yarnpkg.com/braces/-/braces-3.0.3.tgz#490332f40919452272d55a8480adc0c441358789" + integrity sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA== dependencies: - fill-range "^7.0.1" + fill-range "^7.1.1" buffer-crc32@~0.2.3: version "0.2.13" @@ -1622,10 +1622,10 @@ file-entry-cache@^6.0.1: dependencies: flat-cache "^3.0.4" -fill-range@^7.0.1: - version "7.0.1" - resolved "https://registry.yarnpkg.com/fill-range/-/fill-range-7.0.1.tgz#1919a6a7c75fe38b2c7c77e5198535da9acdda40" - integrity sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ== +fill-range@^7.1.1: + version "7.1.1" + resolved "https://registry.yarnpkg.com/fill-range/-/fill-range-7.1.1.tgz#44265d3cac07e3ea7dc247516380643754a05292" + integrity sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg== dependencies: to-regex-range "^5.0.1" From 922e488b23be5fe4b277b0fb3a0278dea88a1d72 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sat, 30 Mar 2024 11:02:49 -0400 Subject: [PATCH 15/20] Extract and re-use element morphing logic Follow-up to [#1185][] Related to [#1192][] The `morph{Page,Frames,Elements}` functions --- Introduce a new `src/core/morphing` module to expose a centralized and re-usable `morphElements(currentElement, newElement)` function to be invoked across the various morphing contexts. Next, move the logic from the `MorphRenderer` into a module-private `IdomorphCallbacks` class. The `IdomorphCallbacks` class (like its `MorphRenderer` predecessor) wraps a call to `Idiomorph` based on its own set of callbacks. The bulk of the logic remains in the `IdomorphCallbacks` class, including checks for `[data-turbo-permanent]`. To serve as a seam for integration, the class retains a reference to a callback responsible for: * providing options for the `Idiomorph` * determining whether or not a node should be skipped while morphing The `MorphingPageRenderer` skips `` elements so that it can override their rendering to use morphing. Similarly, the `MorphingFrameRenderer` provides the `morphStyle: "innerHTML"` option to morph its children. Changes to the renderers --- To integrate with the new module, first rename the `MorphRenderer` to `MorphingPageRenderer` to set a new precedent that communicates the level of the document the morphing is scoped to. With that change in place, define the static `MorphingPageRenderer.renderElement` to mirror the other existing renderer static functions (like [PageRenderer.renderElement][], [ErrorRenderer.renderElement][], and [FrameRenderer.renderElement][]). This integrates with the changes proposed in [#1028][]. Next, modify the rest of the `MorphingPageRenderer` to integrate with its `PageRenderer` ancestor in a way that invokes the static `renderElement` function. This involves overriding the `preservingPermanentElements(callback)` method. In theory, morphing has implications on the concept of "permanence". In practice, morphing has the `[data-turbo-permanent]` attribute receive special treatment during morphing. Following the new precedent, introduce a new `MorphingFrameRenderer` class to define the `MorphingFrameRenderer.renderElement` function that invokes the `morphElements` function with `newElement.children` and `morphStyle: "innerHTML"`. Changes to the StreamActions --- The extraction of the `morphElements` function makes entirety of the `src/core/streams/actions/morph.js` module redundant. This commit removes that module and invokes `morphElements` directly within the `StreamActions.morph` function. Future possibilities --- In the future, additional changes could be made to expose the morphing capabilities as part of the `window.Turbo` interface. For example, applications could experiment with supporting [Page Refresh-style morphing for pages with different URL pathnames][#1177] by overriding the rendering mechanism in `turbo:before-render`: ```js addEventListener("turbo:before-render", (event) => { const someCriteriaForMorphing = ... if (someCriteriaForMorphing) { event.detail.render = Turbo.morphPage } }) addEventListener("turbo:before-frame-render", (event) => { const someCriteriaForMorphingAFrame = ... if (someCriteriaForMorphingAFrame) { event.detail.render = Turbo.morphFrames } }) ``` [#1185]: https://github.com/hotwired/turbo/pull/1185#discussion_r1525281450 [#1192]: https://github.com/hotwired/turbo/pull/1192 [PageRenderer.renderElement]: https://github.com/hotwired/turbo/blob/9fb05e3ed3ebb15fe7b13f52941f25df425e3d15/src/core/drive/page_renderer.js#L5-L11 [ErrorRenderer.renderElement]: https://github.com/hotwired/turbo/blob/9fb05e3ed3ebb15fe7b13f52941f25df425e3d15/src/core/drive/error_renderer.js#L5-L9 [FrameRenderer.renderElement]: https://github.com/hotwired/turbo/blob/9fb05e3ed3ebb15fe7b13f52941f25df425e3d15/src/core/frames/frame_renderer.js#L5-L16 [#1028]: https://github.com/hotwired/turbo/pull/1028 [#1177]: https://github.com/hotwired/turbo/issues/1177 --- src/core/drive/morph_renderer.js | 122 --------------------- src/core/drive/morphing_page_renderer.js | 48 ++++++++ src/core/drive/page_view.js | 8 +- src/core/frames/morphing_frame_renderer.js | 14 +++ src/core/morphing.js | 66 +++++++++++ src/core/streams/actions/morph.js | 65 ----------- src/core/streams/stream_actions.js | 8 +- 7 files changed, 138 insertions(+), 193 deletions(-) delete mode 100644 src/core/drive/morph_renderer.js create mode 100644 src/core/drive/morphing_page_renderer.js create mode 100644 src/core/frames/morphing_frame_renderer.js create mode 100644 src/core/morphing.js delete mode 100644 src/core/streams/actions/morph.js diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js deleted file mode 100644 index 11142463e..000000000 --- a/src/core/drive/morph_renderer.js +++ /dev/null @@ -1,122 +0,0 @@ -import { Idiomorph } from "idiomorph/dist/idiomorph.esm.js" -import { dispatch } from "../../util" -import { PageRenderer } from "./page_renderer" - -export class MorphRenderer extends PageRenderer { - async render() { - if (this.willRender) await this.#morphBody() - } - - get renderMethod() { - return "morph" - } - - get shouldAutofocus() { - return false - } - - // Private - - async #morphBody() { - this.#morphElements(this.currentElement, this.newElement) - this.#reloadRemoteFrames() - - dispatch("turbo:morph", { - detail: { - currentElement: this.currentElement, - newElement: this.newElement - } - }) - } - - #morphElements(currentElement, newElement, morphStyle = "outerHTML") { - this.isMorphingTurboFrame = this.#isFrameReloadedWithMorph(currentElement) - - Idiomorph.morph(currentElement, newElement, { - morphStyle: morphStyle, - callbacks: { - beforeNodeAdded: this.#shouldAddElement, - beforeNodeMorphed: this.#shouldMorphElement, - beforeAttributeUpdated: this.#shouldUpdateAttribute, - beforeNodeRemoved: this.#shouldRemoveElement, - afterNodeMorphed: this.#didMorphElement - } - }) - } - - #shouldAddElement = (node) => { - return !(node.id && node.hasAttribute("data-turbo-permanent") && document.getElementById(node.id)) - } - - #shouldMorphElement = (oldNode, newNode) => { - if (oldNode instanceof HTMLElement) { - if (!oldNode.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isFrameReloadedWithMorph(oldNode))) { - const event = dispatch("turbo:before-morph-element", { - cancelable: true, - target: oldNode, - detail: { - newElement: newNode - } - }) - - return !event.defaultPrevented - } else { - return false - } - } - } - - #shouldUpdateAttribute = (attributeName, target, mutationType) => { - const event = dispatch("turbo:before-morph-attribute", { cancelable: true, target, detail: { attributeName, mutationType } }) - - return !event.defaultPrevented - } - - #didMorphElement = (oldNode, newNode) => { - if (newNode instanceof HTMLElement) { - dispatch("turbo:morph-element", { - target: oldNode, - detail: { - newElement: newNode - } - }) - } - } - - #shouldRemoveElement = (node) => { - return this.#shouldMorphElement(node) - } - - #reloadRemoteFrames() { - this.#remoteFrames().forEach((frame) => { - if (this.#isFrameReloadedWithMorph(frame)) { - this.#renderFrameWithMorph(frame) - frame.reload() - } - }) - } - - #renderFrameWithMorph(frame) { - frame.addEventListener("turbo:before-frame-render", (event) => { - event.detail.render = this.#morphFrameUpdate - }, { once: true }) - } - - #morphFrameUpdate = (currentElement, newElement) => { - dispatch("turbo:before-frame-morph", { - target: currentElement, - detail: { currentElement, newElement } - }) - this.#morphElements(currentElement, newElement.children, "innerHTML") - } - - #isFrameReloadedWithMorph(element) { - return element.src && element.refresh === "morph" - } - - #remoteFrames() { - return Array.from(document.querySelectorAll('turbo-frame[src]')).filter(frame => { - return !frame.closest('[data-turbo-permanent]') - }) - } -} diff --git a/src/core/drive/morphing_page_renderer.js b/src/core/drive/morphing_page_renderer.js new file mode 100644 index 000000000..2884a24fb --- /dev/null +++ b/src/core/drive/morphing_page_renderer.js @@ -0,0 +1,48 @@ +import { FrameElement } from "../../elements/frame_element" +import { MorphingFrameRenderer } from "../frames/morphing_frame_renderer" +import { PageRenderer } from "./page_renderer" +import { dispatch } from "../../util" +import { morphElements } from "../morphing" + +export class MorphingPageRenderer extends PageRenderer { + static renderElement(currentElement, newElement) { + morphElements(currentElement, newElement, { + callbacks: { + beforeNodeMorphed: element => !canRefreshFrame(element) + } + }) + + for (const frame of currentElement.querySelectorAll("turbo-frame")) { + if (canRefreshFrame(frame)) refreshFrame(frame) + } + + dispatch("turbo:morph", { detail: { currentElement, newElement } }) + } + + async preservingPermanentElements(callback) { + return await callback() + } + + get renderMethod() { + return "morph" + } + + get shouldAutofocus() { + return false + } +} + +function canRefreshFrame(frame) { + return frame instanceof FrameElement && + frame.src && + frame.refresh === "morph" && + !frame.closest("[data-turbo-permanent]") +} + +function refreshFrame(frame) { + frame.addEventListener("turbo:before-frame-render", ({ detail }) => { + detail.render = MorphingFrameRenderer.renderElement + }, { once: true }) + + frame.reload() +} diff --git a/src/core/drive/page_view.js b/src/core/drive/page_view.js index 6a40a94d9..1bcb04134 100644 --- a/src/core/drive/page_view.js +++ b/src/core/drive/page_view.js @@ -1,7 +1,7 @@ import { nextEventLoopTick } from "../../util" import { View } from "../view" import { ErrorRenderer } from "./error_renderer" -import { MorphRenderer } from "./morph_renderer" +import { MorphingPageRenderer } from "./morphing_page_renderer" import { PageRenderer } from "./page_renderer" import { PageSnapshot } from "./page_snapshot" import { SnapshotCache } from "./snapshot_cache" @@ -16,10 +16,10 @@ export class PageView extends View { } renderPage(snapshot, isPreview = false, willRender = true, visit) { - const shouldMorphPage = this.isPageRefresh(visit) && snapshot.shouldMorphPage - const rendererClass = shouldMorphPage ? MorphRenderer : PageRenderer + const shouldMorphPage = this.isPageRefresh(visit) && this.snapshot.shouldMorphPage + const rendererClass = shouldMorphPage ? MorphingPageRenderer : PageRenderer - const renderer = new rendererClass(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender) + const renderer = new rendererClass(this.snapshot, snapshot, rendererClass.renderElement, isPreview, willRender) if (!renderer.shouldRender) { this.forceReloaded = true diff --git a/src/core/frames/morphing_frame_renderer.js b/src/core/frames/morphing_frame_renderer.js new file mode 100644 index 000000000..fe42065d5 --- /dev/null +++ b/src/core/frames/morphing_frame_renderer.js @@ -0,0 +1,14 @@ +import { FrameRenderer } from "./frame_renderer" +import { morphChildren } from "../morphing" +import { dispatch } from "../../util" + +export class MorphingFrameRenderer extends FrameRenderer { + static renderElement(currentElement, newElement) { + dispatch("turbo:before-frame-morph", { + target: currentElement, + detail: { currentElement, newElement } + }) + + morphChildren(currentElement, newElement) + } +} diff --git a/src/core/morphing.js b/src/core/morphing.js new file mode 100644 index 000000000..dfd21e839 --- /dev/null +++ b/src/core/morphing.js @@ -0,0 +1,66 @@ +import { Idiomorph } from "idiomorph/dist/idiomorph.esm.js" +import { dispatch } from "../util" + +export function morphElements(currentElement, newElement, { callbacks, ...options } = {}) { + Idiomorph.morph(currentElement, newElement, { + ...options, + callbacks: new DefaultIdiomorphCallbacks(callbacks) + }) +} + +export function morphChildren(currentElement, newElement) { + morphElements(currentElement, newElement.children, { + morphStyle: "innerHTML" + }) +} + +class DefaultIdiomorphCallbacks { + #beforeNodeMorphed + + constructor({ beforeNodeMorphed } = {}) { + this.#beforeNodeMorphed = beforeNodeMorphed || (() => true) + } + + beforeNodeAdded = (node) => { + return !(node.id && node.hasAttribute("data-turbo-permanent") && document.getElementById(node.id)) + } + + beforeNodeMorphed = (currentElement, newElement) => { + if (currentElement instanceof Element) { + if (!currentElement.hasAttribute("data-turbo-permanent") && this.#beforeNodeMorphed(currentElement, newElement)) { + const event = dispatch("turbo:before-morph-element", { + cancelable: true, + target: currentElement, + detail: { currentElement, newElement } + }) + + return !event.defaultPrevented + } else { + return false + } + } + } + + beforeAttributeUpdated = (attributeName, target, mutationType) => { + const event = dispatch("turbo:before-morph-attribute", { + cancelable: true, + target, + detail: { attributeName, mutationType } + }) + + return !event.defaultPrevented + } + + beforeNodeRemoved = (node) => { + return this.beforeNodeMorphed(node) + } + + afterNodeMorphed = (currentElement, newElement) => { + if (currentElement instanceof Element) { + dispatch("turbo:morph-element", { + target: currentElement, + detail: { currentElement, newElement } + }) + } + } +} diff --git a/src/core/streams/actions/morph.js b/src/core/streams/actions/morph.js deleted file mode 100644 index 42e655c95..000000000 --- a/src/core/streams/actions/morph.js +++ /dev/null @@ -1,65 +0,0 @@ -import { Idiomorph } from "idiomorph/dist/idiomorph.esm" -import { dispatch } from "../../../util" - -export default function morph(streamElement) { - const morphStyle = streamElement.hasAttribute("children-only") ? "innerHTML" : "outerHTML" - streamElement.targetElements.forEach((element) => { - Idiomorph.morph(element, streamElement.templateContent, { - morphStyle: morphStyle, - callbacks: { - beforeNodeAdded, - beforeNodeMorphed, - beforeAttributeUpdated, - beforeNodeRemoved, - afterNodeMorphed - } - }) - }) -} - -function beforeNodeAdded(node) { - return !(node.id && node.hasAttribute("data-turbo-permanent") && document.getElementById(node.id)) -} - -function beforeNodeRemoved(node) { - return beforeNodeAdded(node) -} - -function beforeNodeMorphed(target, newElement) { - if (target instanceof HTMLElement) { - if (!target.hasAttribute("data-turbo-permanent")) { - const event = dispatch("turbo:before-morph-element", { - cancelable: true, - target, - detail: { - newElement - } - }) - return !event.defaultPrevented - } - return false - } -} - -function beforeAttributeUpdated(attributeName, target, mutationType) { - const event = dispatch("turbo:before-morph-attribute", { - cancelable: true, - target, - detail: { - attributeName, - mutationType - } - }) - return !event.defaultPrevented -} - -function afterNodeMorphed(target, newElement) { - if (newElement instanceof HTMLElement) { - dispatch("turbo:morph-element", { - target, - detail: { - newElement - } - }) - } -} diff --git a/src/core/streams/stream_actions.js b/src/core/streams/stream_actions.js index 486dc8566..48ad92da5 100644 --- a/src/core/streams/stream_actions.js +++ b/src/core/streams/stream_actions.js @@ -1,5 +1,5 @@ import { session } from "../" -import morph from "./actions/morph" +import { morphElements, morphChildren } from "../morphing" export const StreamActions = { after() { @@ -40,6 +40,10 @@ export const StreamActions = { }, morph() { - morph(this) + const morph = this.hasAttribute("children-only") ? + morphChildren : + morphElements + + this.targetElements.forEach((targetElement) => morph(targetElement, this.templateContent)) } } From 9369eb9f57216f029615c1df03c2fbb053f957de Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 4 Apr 2024 15:17:29 -0400 Subject: [PATCH 16/20] Re-structure `turbo-stream[action=morph]` support This commit re-structures the new support for `turbo-stream[action="morph"]` elements introduced in [#1185][]. Since the `` hasn't yet been part of a release, there's an opportunity to rename it without being considerate of backwards compatibility. As an alternative to introduce a new Stream Action, this commit changes existing actions to be more flexible. For example, the `` element behaves like a specialized version of a ``, since it operates on the target element's `outerHTML` property. Similarly, the `` element behaves like a specialized version of a ``, since it operates on the target element's `innerHTML` property. ```diff - + - + ``` This commit removes the `[action="morph"]` support entirely, and re-implements it in terms of the `[action="replace"]` and `[action="update"]` support. By consolidating concepts, the "scope" of the modifications is more clearly communicated to callers that are familiar with the underlying DOM interfaces (`Element.replaceWith` and `Element.innerHTML`) that are invoked by the conventionally established Replace and Update actions. This proposal also aims to reinforce the "method" terminology introduced by the Page Refresh `` element. [#1185]: https://github.com/hotwired/turbo/pull/1185 --- src/core/streams/stream_actions.js | 28 ++++++----- src/tests/fixtures/morph_stream_action.html | 16 ------- src/tests/fixtures/stream.html | 4 ++ .../functional/morph_stream_action_tests.js | 48 ------------------- src/tests/functional/stream_tests.js | 46 +++++++++++++++++- src/tests/unit/stream_element_tests.js | 15 +++--- 6 files changed, 74 insertions(+), 83 deletions(-) delete mode 100644 src/tests/fixtures/morph_stream_action.html delete mode 100644 src/tests/functional/morph_stream_action_tests.js diff --git a/src/core/streams/stream_actions.js b/src/core/streams/stream_actions.js index 48ad92da5..a038add0d 100644 --- a/src/core/streams/stream_actions.js +++ b/src/core/streams/stream_actions.js @@ -25,25 +25,31 @@ export const StreamActions = { }, replace() { - this.targetElements.forEach((e) => e.replaceWith(this.templateContent)) + const method = this.getAttribute("method") + + this.targetElements.forEach((targetElement) => { + if (method === "morph") { + morphElements(targetElement, this.templateContent) + } else { + targetElement.replaceWith(this.templateContent) + } + }) }, update() { + const method = this.getAttribute("method") + this.targetElements.forEach((targetElement) => { - targetElement.innerHTML = "" - targetElement.append(this.templateContent) + if (method === "morph") { + morphChildren(targetElement, this.templateContent) + } else { + targetElement.innerHTML = "" + targetElement.append(this.templateContent) + } }) }, refresh() { session.refresh(this.baseURI, this.requestId) - }, - - morph() { - const morph = this.hasAttribute("children-only") ? - morphChildren : - morphElements - - this.targetElements.forEach((targetElement) => morph(targetElement, this.templateContent)) } } diff --git a/src/tests/fixtures/morph_stream_action.html b/src/tests/fixtures/morph_stream_action.html deleted file mode 100644 index df91274f5..000000000 --- a/src/tests/fixtures/morph_stream_action.html +++ /dev/null @@ -1,16 +0,0 @@ - - - - - Morph Stream Action - - - - - - -
-
Morph me
-
- - diff --git a/src/tests/fixtures/stream.html b/src/tests/fixtures/stream.html index 85e8c94e4..a2b78907d 100644 --- a/src/tests/fixtures/stream.html +++ b/src/tests/fixtures/stream.html @@ -39,5 +39,9 @@
+ +
+
Morph me
+
diff --git a/src/tests/functional/morph_stream_action_tests.js b/src/tests/functional/morph_stream_action_tests.js deleted file mode 100644 index b4f04c9d7..000000000 --- a/src/tests/functional/morph_stream_action_tests.js +++ /dev/null @@ -1,48 +0,0 @@ -import { test, expect } from "@playwright/test" -import { nextEventOnTarget, noNextEventOnTarget } from "../helpers/page" - -test("dispatches a turbo:before-morph-element & turbo:morph-element for each morph stream action", async ({ page }) => { - await page.goto("/src/tests/fixtures/morph_stream_action.html") - - await page.evaluate(() => { - window.Turbo.renderStreamMessage(` - - - - `) - }) - - await nextEventOnTarget(page, "message_1", "turbo:before-morph-element") - await nextEventOnTarget(page, "message_1", "turbo:morph-element") - await expect(page.locator("#message_1")).toHaveText("Morphed") -}) - -test("preventing a turbo:before-morph-element prevents the morph", async ({ page }) => { - await page.goto("/src/tests/fixtures/morph_stream_action.html") - - await page.evaluate(() => { - addEventListener("turbo:before-morph-element", (event) => { - event.preventDefault() - }) - }) - - await page.evaluate(() => { - window.Turbo.renderStreamMessage(` - - - - `) - }) - - await nextEventOnTarget(page, "message_1", "turbo:before-morph-element") - await noNextEventOnTarget(page, "message_1", "turbo:morph-element") - await expect(page.locator("#message_1")).toHaveText("Morph me") -}) diff --git a/src/tests/functional/stream_tests.js b/src/tests/functional/stream_tests.js index 40f464d21..5c77016d8 100644 --- a/src/tests/functional/stream_tests.js +++ b/src/tests/functional/stream_tests.js @@ -1,9 +1,11 @@ -import { test } from "@playwright/test" +import { expect, test } from "@playwright/test" import { assert } from "chai" import { hasSelector, nextBeat, nextEventNamed, + nextEventOnTarget, + noNextEventOnTarget, readEventLogs, waitUntilNoSelector, waitUntilText @@ -182,6 +184,48 @@ test("receiving a remove stream message preserves focus blurs the activeElement" assert.notOk(await hasSelector(page, ":focus")) }) +test("dispatches a turbo:before-morph-element & turbo:morph-element for each morph stream action", async ({ page }) => { + await page.evaluate(() => { + window.Turbo.renderStreamMessage(` + + + + `) + }) + + await nextEventOnTarget(page, "message_1", "turbo:before-morph-element") + await nextEventOnTarget(page, "message_1", "turbo:morph-element") + await expect(page.locator("#message_1")).toHaveText("Morphed") +}) + +test("preventing a turbo:before-morph-element prevents the morph", async ({ page }) => { + await page.evaluate(() => { + addEventListener("turbo:before-morph-element", (event) => { + event.preventDefault() + }) + }) + + await page.evaluate(() => { + window.Turbo.renderStreamMessage(` + + + + `) + }) + + await nextEventOnTarget(page, "message_1", "turbo:before-morph-element") + await noNextEventOnTarget(page, "message_1", "turbo:morph-element") + await expect(page.locator("#message_1")).toHaveText("Morph me") +}) + async function getReadyState(page, id) { return page.evaluate((id) => { const element = document.getElementById(id) diff --git a/src/tests/unit/stream_element_tests.js b/src/tests/unit/stream_element_tests.js index 0d3e04b61..455581607 100644 --- a/src/tests/unit/stream_element_tests.js +++ b/src/tests/unit/stream_element_tests.js @@ -5,11 +5,12 @@ import { assert } from "@open-wc/testing" import { sleep } from "../helpers/page" import * as Turbo from "../../index" -function createStreamElement(action, target, templateElement) { +function createStreamElement(action, target, templateElement, attributes = {}) { const element = new StreamElement() if (action) element.setAttribute("action", action) if (target) element.setAttribute("target", target) if (templateElement) element.appendChild(templateElement) + Object.entries(attributes).forEach((attribute) => element.setAttribute(...attribute)) return element } @@ -197,9 +198,9 @@ test("test action=refresh discarded when matching request id", async () => { assert.ok(document.body.hasAttribute("data-modified")) }) -test("action=morph", async () => { +test("action=replace method=morph", async () => { const templateElement = createTemplateElement(`

Hello Turbo Morphed

`) - const element = createStreamElement("morph", "hello", templateElement) + const element = createStreamElement("replace", "hello", templateElement, { method: "morph" }) assert.equal(subject.find("div#hello")?.textContent, "Hello Turbo") @@ -210,9 +211,9 @@ test("action=morph", async () => { assert.equal(subject.find("h1#hello")?.textContent, "Hello Turbo Morphed") }) -test("action=morph with text content change", async () => { +test("action=replace method=morph with text content change", async () => { const templateElement = createTemplateElement(`
Hello Turbo Morphed
`) - const element = createStreamElement("morph", "hello", templateElement) + const element = createStreamElement("replace", "hello", templateElement, { method: "morph" }) assert.equal(subject.find("div#hello")?.textContent, "Hello Turbo") @@ -223,9 +224,9 @@ test("action=morph with text content change", async () => { assert.equal(subject.find("div#hello")?.textContent, "Hello Turbo Morphed") }) -test("action=morph children-only", async () => { +test("action=update method=morph", async () => { const templateElement = createTemplateElement(`

Hello Turbo Morphed

`) - const element = createStreamElement("morph", "hello", templateElement) + const element = createStreamElement("update", "hello", templateElement, { method: "morph" }) const target = subject.find("div#hello") assert.equal(target?.textContent, "Hello Turbo") element.setAttribute("children-only", true) From c591267465f1535107afb121f5aa26ba4cac2274 Mon Sep 17 00:00:00 2001 From: Nick Pezza Date: Wed, 7 Feb 2024 12:07:54 -0500 Subject: [PATCH 17/20] Ensure fetch method is always uppercase --- src/http/fetch_request.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/http/fetch_request.js b/src/http/fetch_request.js index f4ff5adbe..f39fd8101 100644 --- a/src/http/fetch_request.js +++ b/src/http/fetch_request.js @@ -56,7 +56,7 @@ export class FetchRequest { this.fetchOptions = { credentials: "same-origin", redirect: "follow", - method: method, + method: method.toUpperCase(), headers: { ...this.defaultHeaders }, body: body, signal: this.abortSignal, @@ -79,7 +79,7 @@ export class FetchRequest { this.url = url this.fetchOptions.body = body - this.fetchOptions.method = fetchMethod + this.fetchOptions.method = fetchMethod.toUpperCase() } get headers() { From 64680d79f82cb0617bb8e46f80ad64b630132043 Mon Sep 17 00:00:00 2001 From: Nick Pezza Date: Wed, 7 Feb 2024 12:14:43 -0500 Subject: [PATCH 18/20] Fix tests --- src/tests/functional/form_submission_tests.js | 8 ++++---- src/tests/functional/visit_tests.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tests/functional/form_submission_tests.js b/src/tests/functional/form_submission_tests.js index e7447944f..32d9366dd 100644 --- a/src/tests/functional/form_submission_tests.js +++ b/src/tests/functional/form_submission_tests.js @@ -188,7 +188,7 @@ test("supports transforming a POST submission to a GET in a turbo:submit-start l test("supports transforming a GET submission to a POST in a turbo:submit-start listener", async ({ page }) => { await page.evaluate(() => addEventListener("turbo:submit-start", (({ detail }) => { - detail.formSubmission.method = "post" + detail.formSubmission.method = "POST" detail.formSubmission.body.set("path", "/src/tests/fixtures/one.html") detail.formSubmission.body.set("greeting", "Hello, from an event listener") })) @@ -992,7 +992,7 @@ test("link method form submission submits a single request", async ({ page }) => const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request") assert.ok(await noNextEventNamed(page, "turbo:before-fetch-request")) - assert.equal(fetchOptions.method, "post", "[data-turbo-method] overrides the GET method") + assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method") assert.equal(requestCounter, 1, "submits a single HTTP request") }) @@ -1006,7 +1006,7 @@ test("link method form submission inside frame submits a single request", async const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request") assert.ok(await noNextEventNamed(page, "turbo:before-fetch-request")) - assert.equal(fetchOptions.method, "post", "[data-turbo-method] overrides the GET method") + assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method") assert.equal(requestCounter, 1, "submits a single HTTP request") }) @@ -1020,7 +1020,7 @@ test("link method form submission targeting frame submits a single request", asy const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request") assert.ok(await noNextEventNamed(page, "turbo:before-fetch-request")) - assert.equal(fetchOptions.method, "post", "[data-turbo-method] overrides the GET method") + assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method") assert.equal(requestCounter, 2, "submits a single HTTP request then follows a redirect") }) diff --git a/src/tests/functional/visit_tests.js b/src/tests/functional/visit_tests.js index 782871747..dd0b97968 100644 --- a/src/tests/functional/visit_tests.js +++ b/src/tests/functional/visit_tests.js @@ -106,7 +106,7 @@ test("turbo:before-fetch-request event.detail", async ({ page }) => { await page.click("#same-origin-link") const { url, fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request") - assert.equal(fetchOptions.method, "get") + assert.equal(fetchOptions.method, "GET") assert.ok(url.includes("/src/tests/fixtures/one.html")) }) From b931708dd3890d40e09400e786c98df3d92eb32b Mon Sep 17 00:00:00 2001 From: Nick Pezza Date: Sat, 13 Jul 2024 11:13:45 -0400 Subject: [PATCH 19/20] Tweak setting fetchmethod --- src/observers/link_prefetch_observer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 6265f075a..04c583ab1 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -93,7 +93,7 @@ export class LinkPrefetchObserver { } #tryToUsePrefetchedRequest = (event) => { - if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") { + if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "GET") { const cached = prefetchCache.get(event.detail.url.toString()) if (cached) { From f88bfe45ed44abb6ea9140a8ba138dfe68aa2730 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 19 Jul 2024 06:41:19 +0200 Subject: [PATCH 20/20] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7488a73fd..28ef82bff 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@hotwired/turbo", - "version": "8.0.4", + "version": "8.0.5", "description": "The speed of a single-page web application without having to write any JavaScript", "module": "dist/turbo.es2017-esm.js", "main": "dist/turbo.es2017-umd.js",