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

Automatically inserts Turbo Stream responses #6

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

excid3
Copy link
Contributor

@excid3 excid3 commented Jun 3, 2021

The user may not realize this how to execute Turbo Stream actions in the response, so it is nice if we can take care of it for them. By automatically inserting the Turbo Stream responses into the body, we can apply those operations automatically for the user.

I couldn't think of any situations where you might want to disable this, but we could add an option to disable it if needed.

Thoughts on this or how it can be improved?

cc @dhh

src/response.js Outdated Show resolved Hide resolved
@excid3 excid3 force-pushed the handle-turbo-stream-on-success branch 2 times, most recently from 67d5b22 to 7fb5b3d Compare June 4, 2021 14:43
This makes it easier for users so they don't have to insert the Turbo
Stream HTML into the body.
@excid3 excid3 force-pushed the handle-turbo-stream-on-success branch from 7fb5b3d to f28a6d5 Compare June 4, 2021 14:44
@excid3 excid3 requested a review from marcelolx June 4, 2021 14:44
Copy link
Collaborator

@marcelolx marcelolx left a comment

Choose a reason for hiding this comment

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

Thank you @excid3!

@marcelolx marcelolx merged commit c8d7881 into rails:main Jun 4, 2021
@excid3 excid3 deleted the handle-turbo-stream-on-success branch June 4, 2021 14:48
@excid3
Copy link
Contributor Author

excid3 commented Jun 4, 2021

@marcelolx Thanks! ✌️❤️

On a related note... do you think we should modify Turbo to automatically set window.Turbo? If Request.js and iOS/Android all need it, seems like it would make sense to out of the box.

@dhh
Copy link
Member

dhh commented Jun 4, 2021

Yes, definitely want to set window.Turbo. We have the same issue at Basecamp. In fact, we do that in our pack setup:

const { Turbo } = require("@hotwired/turbo-rails")
window.Turbo = Turbo

@marcelolx
Copy link
Collaborator

@excid3 @dhh I'll take a look on that and send a PR to turbo

@excid3
Copy link
Contributor Author

excid3 commented Jun 4, 2021

@marcelolx I started one already, just need to test it. hotwired/turbo#280

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

Successfully merging this pull request may close these issues.

3 participants