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

adsf-live sends the path of the changed file instead of the path of the web page #37

Closed
Fjan opened this issue Apr 4, 2024 · 4 comments
Labels

Comments

@Fjan
Copy link
Contributor

Fjan commented Apr 4, 2024

Steps to reproduce

  1. Use nanoc with adsf-live
  2. Make a change to a page

Expected behavior

adsf-live should send the path of the web page to live reload.

Actual behavior

adsf-live sends the path to the file here
path: "#{Dir.pwd}#{path}"

instead of path: path

Details

You typically only want to reload the page if the change occurred on the page you are viewing. When you change a layout in nanoc it will send many changed pages over the web socket. For livereload to know if the current page changed it needs to compare document.location.pathname with path. At this time it always reloads even if no match is found, but that results in ugly flickering if many pages change. It's not hard to modify the livereload.js to only reload if the path matches, but this comparison is needlessly complicated because the file path is being sent, from which the path first needs to be extracted.

@denisdefreyne
Copy link
Owner

Ooh, I’ve seen this behavior but never managed to figure out what was causing it. Good catch!

@Fjan
Copy link
Contributor Author

Fjan commented Apr 4, 2024

To be clear, I also needed to make a small change to the livereload.js included with rack-livereload to completely fix the issue, but that fix will be much easier if adsf-live sends the path

(If you use your own copy of livereload and change this line to become

if (path == document.location.pathname) return this.reloadPage()

then the reloading works properly with the change suggested above)

@denisdefreyne
Copy link
Owner

See #38 — This is now merged (see e2b15cd). I’ll do a release soon. Thank you!

@denisdefreyne
Copy link
Owner

This is fixed in the adsf 1.5.1 release.

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

No branches or pull requests

2 participants