-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
3 reload modes: hard, soft, css-only. Reload on error pages. #34
base: main
Are you sure you want to change the base?
Conversation
@elik-ru thanks for the PR 🙌 I will review and test on the weekend |
Nice PR. Tested at my end, the only thing that didnt work out was the default It fails silently here because my stylesheet tag do not contain a stylesheet data attribute, as your code is expecting. My link is: <link rel="stylesheet" href="/assets/application-{{big-hash-here}}.css" data-turbo-track="reload"> I could fix it adding the data attribute manually, like this: # application.html.erb
<%= stylesheet_link_tag "application", "data-turbo-track": "reload", "data-stylesheet": "application.css" %> Is this expected? |
@LEstradioto Thanks for your feedback! |
Hi @kirillplatonov! How is it going? Can i help you in somehow to make it easier to review? |
@elik-ru could you please extract reloading on error pages into separate PR? It would be easier to review and merge this way |
@kirillplatonov There 2 commits for 2 features, but seems some improvements leaked from one to the other. |
Continuation: #46 |
This PR introduce 2 major changes:
Reload modes:
Depending on what changed we can do different reload types.
Auto-reload on error pages: I have added a middleware which injects livereload code into error pages.
To be honest, i did this couple of month ago and did not have time to submit a PR, and now looking at the code it looks like an alien wrote it. But if you think i need to improve something in this PR, i'll try to do this.