Skip to content
New issue

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

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

Already on GitHub? # to your account

Replacing Turbolinks with @hotwired/turbo #726

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rmarronnier
Copy link

@jwoertink
Copy link
Member

Not sure why all of the CI is failing, but I've added this in to the website luckyframework/website#932 We can test on a live site that it's fine before making the final merge.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

We will need the HTML head tags updated as well

css_link asset("css/app.css"), data_turbolinks_track: "reload"
js_link asset("js/app.js"), defer: "true", data_turbolinks_track: "reload"
meta name: "turbolinks-cache-control", content: "no-cache"

@rmarronnier
Copy link
Author

rmarronnier commented Feb 23, 2022

Yes, the CI is failing and because the global specs using the support should_run_successfully method don't return the "downstream" error text, I'm blind. (Problem A)

We can test on a live site that it's fine before making the final merge.

Maybe it's enough, but I wanted to create specific lucky flowspecs to check we have the same behavior between before (@rails/ujs + turbolinks) and after (@hotwired/turbo only). (Problem B)

Right now, only the browser_authentification_app_skeleton uses lucky_flow, and for specific auth logic.

I have to tackle problem A before working on problem B, or I'll tiptoe in the dark, wondering why the specs failed.

Also for reference/note to myself :

https://github.com/hotwired/turbo-rails/blob/main/UPGRADING.md
https://www.honeybadger.io/blog/hb-turbolinks-to-turbo/
https://discuss.hotwired.dev/t/migrating-from-ujs-to-turbo/1942

I'll put this PR as draft because, it's not ready as it is. I don't have bandwidth right now to work on this but anyone is welcome to continue this :-)

Oups ! Sorry for the accidental closing !

@rmarronnier rmarronnier reopened this Feb 23, 2022
@rmarronnier rmarronnier marked this pull request as draft February 23, 2022 19:34
@jwoertink
Copy link
Member

Looks like this just became a bit larger of a PR:
A comparable version of this handler would need to be created:
https://github.com/luckyframework/lucky/blob/main/src/lucky/redirectable_turbolinks_support.cr

Then whatever needs to be done to ensure this stuff works luckyframework/website#947

Just dropping these links here so we have a spot to reference them.

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

Successfully merging this pull request may close these issues.

2 participants