Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-structure turbo-stream[action=morph] support #1240

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Apr 4, 2024

Closes #1163
Follow-up to #1185

turbo-site PR

This commit re-structures the new support for
turbo-stream[action="morph"] elements introduced in #1185.

Since the <turbo-stream action="morph"> 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 <turbo-stream method="morph"> element behaves like a specialized version of a <turbo-stream method="replace">, since it operates on the target element's outerHTML property.

Similarly, the <turbo-stream method="morph" children-only> element behaves like a specialized version of a <turbo-stream method="update">, since it operates on the target element's innerHTML property.

-<turbo-stream action="morph">
+<turbo-stream action="replace" method="morph">
   <template>Replace me with morphing</template>
 </turbo-stream>

-<turbo-stream action="morph" children-only>
+<turbo-stream action="update" method="morph">
   <template>Update me with morphing</template>
 </turbo-stream>

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 <meta name="turbo-refresh-method" content="morph"> element.

seanpdoyle added a commit to seanpdoyle/turbo-site that referenced this pull request Apr 4, 2024
Related to [hotwired/turbo#1240][]

Since the `<turbo-stream action="morph">` 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 `<turbo-stream method="morph">` element behaves like a
specialized version of a `<turbo-stream method="replace">`, since it
operates on the target element's `outerHTML` property.

Similarly, the `<turbo-stream method="morph" children-only>` element
behaves like a specialized version of a `<turbo-stream
method="update">`, since it operates on the target element's `innerHTML`
property.

```diff
-<turbo-stream action="morph">
+<turbo-stream action="replace" method="morph">
   <template>Replace me with morphing</template>
 </turbo-stream>

-<turbo-stream action="morph" children-only>
+<turbo-stream action="update" method="morph">
   <template>Update me with morphing</template>
 </turbo-stream>
```

[hotwired/turbo#1240]: hotwired/turbo#1240
@seanpdoyle
Copy link
Contributor Author

I started to open a PR related to hotwired/turbo-rails#583 to remove turbo_stream.morph with extension to its turbo_stream.replace and turbo_stream.update counterparts, but wasn't sure whether or not it'd be worth removing those methods.

For example, the turbo_stream.morph method could serve as an abstraction on top of building a <turbo-stream action="replace" method="morph"> in the same way as a turbo_stream.morph(..., children_only: true) call could be an abstraction on top of building a <turbo-stream action="update" method="morph">.

If this PR's proposal is well-received, I'll open a corresponding PR to turbo-rails.

@seanpdoyle
Copy link
Contributor Author

After doing some more research, I see that the original implementation introduced in #1185 intentionally overlaps with the turbo-stream[action="morph"] element provided by the turbo-morph package.

Since that package integrates with morphdom, support for the [children-only] attribute is a natural corollary to the childrenOnly: Boolean option that Morphdom supports.

Since Turbo 8's morphing is powered by Idiomorph, drawing inspiration from the Morphdom API isn't as compelling as it is for the turbo-morph package.

Instead, utilizing existing Turbo terminology (instead of something like [morph-style] to mirror Idiomorph's morphStyle: "innerHTML" | "outerHTML" option) encapsulates that implementation detail in a way that makes it possible to change the underlying morphing library in the future without affecting downstream applications.

@seanpdoyle
Copy link
Contributor Author

@jorgemanrubia does this feel like an improvement?

@jonsgreen
Copy link

jonsgreen commented May 9, 2024

Not sure that my 2 cents are of value here but I do like the idea of having a different api for stream morphing that would not conflict with the turbo-power library. There are other features in turbo-power that are of value but I would prefer to use turbo with Idiomorph for morphing on a page and I believe that if turbo_rails were to use turbo_stream.morph it would collide with turbo-power.

@seanpdoyle, I am curious if you have a sense of when this feature might be released? As noted in #1229 it is in the docs but not actually functional yet which tripped me up. As other folks have mentioned in #1185 I have a pattern that I use in an app in which I post back to the same action (e.g. new or edit) to update the state of an object but without persisting it to the database so a redirect is not possible. I would like the UI to adjust as the state of the form object changes without doing a full replace since that would lose focus so morphing would be ideal.

I think I did at least finally figure out how to get the full page render to trigger a morph refresh by passing a status code other than 200. Does that sound right? Tricky.

@jorgemanrubia
Copy link
Member

I love this @seanpdoyle. It makes the API tighter and more consistent. Thanks 🙏 . I would imagine this will require some adjustments after merging #1234. Happy to tackle this one next. Thanks.

@jorgemanrubia
Copy link
Member

jorgemanrubia commented Jul 12, 2024

@seanpdoyle if you can rebase and adapt to the last changes I'll merge. Thanks!

This commit re-structures the new support for
`turbo-stream[action="morph"]` elements introduced in [hotwired#1185][].

Since the `<turbo-stream action="morph">` 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 `<turbo-stream method="morph">` element behaves like a
specialized version of a `<turbo-stream method="replace">`, since it
operates on the target element's `outerHTML` property.

Similarly, the `<turbo-stream method="morph" children-only>` element
behaves like a specialized version of a `<turbo-stream
method="update">`, since it operates on the target element's `innerHTML`
property.

```diff
-<turbo-stream action="morph">
+<turbo-stream action="replace" method="morph">
   <template>Replace me with morphing</template>
 </turbo-stream>

-<turbo-stream action="morph" children-only>
+<turbo-stream action="update" method="morph">
   <template>Update me with morphing</template>
 </turbo-stream>
```

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 `<meta name="refresh-method" content="morph">`
element.

[hotwired#1185]: hotwired#1185
seanpdoyle added a commit to seanpdoyle/turbo-site that referenced this pull request Jul 12, 2024
Related to [hotwired/turbo#1240][]

Since the `<turbo-stream action="morph">` 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 `<turbo-stream method="morph">` element behaves like a
specialized version of a `<turbo-stream method="replace">`, since it
operates on the target element's `outerHTML` property.

Similarly, the `<turbo-stream method="morph" children-only>` element
behaves like a specialized version of a `<turbo-stream
method="update">`, since it operates on the target element's `innerHTML`
property.

```diff
-<turbo-stream action="morph">
+<turbo-stream action="replace" method="morph">
   <template>Replace me with morphing</template>
 </turbo-stream>

-<turbo-stream action="morph" children-only>
+<turbo-stream action="update" method="morph">
   <template>Update me with morphing</template>
 </turbo-stream>
```

[hotwired/turbo#1240]: hotwired/turbo#1240
@seanpdoyle
Copy link
Contributor Author

@jorgemanrubia I've rebased. I've also opened hotwired/turbo-site#186 to document this change.

@omarluq
Copy link
Contributor

omarluq commented Jul 12, 2024

@seanpdoyle Im happy to take on the refactor on the turbo-rails side if you haven't started it yet

cc: @jorgemanrubia

@seanpdoyle
Copy link
Contributor Author

@omarluq I hadn't started anything on the Rails side.

Go for it! Thank you for the help.

Copy link
Member

@jorgemanrubia jorgemanrubia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @seanpdoyle. This is a great change 👍 .

@jorgemanrubia jorgemanrubia merged commit a4f3a90 into hotwired:main Jul 12, 2024
1 check passed
@seanpdoyle seanpdoyle deleted the rename-morph-stream-action branch July 12, 2024 19:01
@searls
Copy link

searls commented Jul 15, 2024

Reading the play-by-play, this change makes a lot of sense to me. Update and Replace are the actions. morph vs replace are the methods by which that's accomplished. Comingling those interests into a single category (turbo stream actions) would have resulted in a bunch of confusion, option duplication, etc.

@marcoroth
Copy link
Member

marcoroth commented Jul 16, 2024

Would it make sense to keep the morph action around as a shim and emit a warning saying that this action was "deprecated" in favor of using replace with method=morph?

@brunoprietog
Copy link
Collaborator

I don't think so, because no version was released with the Turbo stream morph action

@searls
Copy link

searls commented Jul 16, 2024

@marcoroth, my understanding was it was never actually included in a release (the fact it didn't work is how I found these issues initially)

@marcoroth
Copy link
Member

Oh right, nevermind then. I guess I confused it with the refresh action which has nothing to do with this.

@jorgemanrubia
Copy link
Member

jorgemanrubia commented Jul 19, 2024

I just released turbo v8.0.5 and turbo-rails v2.0.6. I'll merge those site docs next.

@searls
Copy link

searls commented Jul 20, 2024

Thanks! I'll try incorporating this next week and may update this blog post I recently wrote

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add morph Turbo Stream Action
7 participants