-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Improve editing experience when using --livereload
#3391
Conversation
JavaScript master Martin 🙇 impressive |
Thanks! |
Actually let me just check out this PR directly. That makes more sense based on what I observed: #3389 (comment) Thanks a lot! |
It's a great change and seems to be working great. |
Yes, as I mentioned. The thing is: if you're working on any page, the currently active page will reload. The other pages will only reload once you focus them again, yes, but as long as the current page reloads as long as you're working on it, it's the exact same information as before. I've been working with those changes all day and have to say it is so much better than before. Much faster, and I don't have to restart MkDocs again and again when the server starts hanging. Try working a little with it, I think you'll eventually agree that the new behavior is better. If not, you can turn down this PR, but we then need to fix the hanging livereload server someway else. |
Co-authored-by: Oleh Prypin <oleh@pryp.in>
Another question: why does this have to wait for MkDocs 1.6.0? It's essentially a bugfix. Other than that, do you have plans when MkDocs 1.6.0 will be out? If I look at the milestone, it seems that there are currently 9 open issues that should go into 1.6.0, several of which are likely to take longer as they're quite substantial changes, particularly:
Thus, work on 1.6.0 hasn't really started yet, except for cache busting which was a linked PR. Hoever, IMHO, cache busting is something that should not go into MkDocs, as it is very hard to get right if you do it right. We had a long conversation on this topic on Gitter some time ago, where I shared my concerns to implement this into MkDocs. |
Because it is possible for it negatively affect workflows:
So not a straightforward decision |
But this will definitely be in 1.6.0, or maybe still in the earlier release |
I don't understand why a small bugfix that improves the developer experience significantly has to wait for months until the rest of the issues was tackled, all of which have probably deeper implications and consequences, but okay then. I'm not saying that the code is free of bugs or that I'm 100% sure that we caught all conditions, but we can fix stuff in subsequent releases. I'm also not saying we should release it now, we should first test it a few days before we're confident that it works. Releasing bugfixes a little faster wouldn't hurt the project, au contraire. Especially since there's funding now. |
Again as I was saying, this isn't a pure bugfix: I have described a downside to this that's unarguably there, the only argument is whether it's important enough. |
The "downside" is a change in behavior. Every release is a change in behavior, that's what maintaining and developing software is all about. If users complain about it, we can reconsider if we can somehow replicate the previous behavior or even revert to the previous behavior, but |
I personally cannot find a use-case where I would need inactive tabs to be reloaded. When I work on docs, I usually have a a single docs tab open, and I only need it reloaded when I'm on it. I suppose that 99.5% of MkDocs users use Also, I've seen MkDocs "block" many times after I triggered a reload (saving a file), with a single tab open. Usually I simply Ctrl-C and restart the server. I understand there's also #3390 that fixes this particular issue, but if #3391 (this PR) fixes it as well, and fixes more things, then I'd vote to merge it instead of #3390 for 1.5.3. Also also, IMO this is not super critical either, so I wouldn't mind waiting a bit longer. It would be sad to have to wait months though. Not sure what's the ETA for 1.6.0. And I understand that heavier users of My 2 cents! |
Likewise, while I'm satisfied with the actual bug fix in #3390 I must admit that I don't mind going straight for #3391 as well if we all agree about it (and I don't mind waiting for 1.6 on the other hand). This may be personal and biased but I don't like having websites update their views "in the background" while I'm not looking at them. It's sane and more resource efficient to have the tabs reload themselves only when focused again. For the users really in need of that feature there are browser extensions out there anyway so it's not like we'll be hard breaking things IMHO. |
eaea444
to
beb444f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can logically read, this looks good and cool 👍
Friendly ping to ask when 1.6.0 will be released. It's now almost been 4 months since this fix was proposed, and 3 months since it has been merged. It would make my life (and possibly the life of others) much easier. If any bugs with this code are encountered, you can happily assign them to me and I'll fix them quickly. |
Friendly ping 7 weeks after my last. Could we please get this released as a patch release, maybe 1.5.4? Is there anything I can help with to get this out the door? The editing experience is quite annoying. |
Closes #3389 and supersedes/closes #3390.
I've refactored the livereload functionality. It's more predictable now and solves another problem: it's currently not possible to open more than 6 tabs, because browsers are limited to 10 connections per destination, and the livereload connections queue up and block further connections. The problem is that every tab will keep an active livereload connection, regardless of whether it is visible or not.
This makes working on several pages in parallel impossible – see the never ending loading indicators here:
This PR addresses this issue and makes livereload more efficient.
poll
andstop
, that are called when certain events happenThe limitation on the number of open pages is gone:
If you switch back and forth between two tabs, you can inspect the network and see the new behavior:
IMHO, there's no upside in keeping livereload connections for inactive tabs alive. This implementation improves the editing experience, as it is sometimes necessary to open more than 5 or 6 pages simultaneously☺️