From c9dbc34824b9c2537d9add3fb1e6d74a1c670716 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 4 Apr 2024 15:17:29 -0400 Subject: [PATCH] Re-structure `turbo-stream[action=morph]` support This commit re-structures the new support for `turbo-stream[action="morph"]` elements introduced in [#1185][]. 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. 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/actions/morph.js | 31 +++++++----- src/core/streams/stream_actions.js | 26 ++++++---- 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 +++--- 7 files changed, 93 insertions(+), 93 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/actions/morph.js b/src/core/streams/actions/morph.js index 42e655c95..7680c4f63 100644 --- a/src/core/streams/actions/morph.js +++ b/src/core/streams/actions/morph.js @@ -1,19 +1,24 @@ 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 - } - }) +export function morphElement(target, element) { + idiomorph(target, element, { morphStyle: "outerHTML" }) +} + +export function morphChildren(target, childElements) { + idiomorph(target, childElements, { morphStyle: "innerHTML" }) +} + +function idiomorph(target, element, options = {}) { + Idiomorph.morph(target, element, { + ...options, + callbacks: { + beforeNodeAdded, + beforeNodeMorphed, + beforeAttributeUpdated, + beforeNodeRemoved, + afterNodeMorphed + } }) } diff --git a/src/core/streams/stream_actions.js b/src/core/streams/stream_actions.js index 486dc8566..f62dd3336 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 { morphChildren, morphElement } from "./actions/morph" export const StreamActions = { after() { @@ -25,21 +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") { + morphElement(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() { - morph(this) } } 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)