qwik
qwik copied to clipboard
feat: add qnavigatestart and qnavigateend events #2202
Signed-off-by: Jeremy Wickersheimer [email protected]
What is it?
- [X] Feature / enhancement
- [ ] Bug
- [ ] Docs / tests
Description
Implement the events for issue #2202
Use cases and why
- makes better UX possible as developpers can now hook up to those events and provide UI feedback like a loader, message if the load is long, etc ...
Checklist:
- [X] My code follows the developer guidelines of this project
- [X] I have performed a self-review of my own code
- [ ] I have made corresponding changes to the documentation
- [ ] Added new tests to cover the fix / functionality
Run & review this pull request in StackBlitz Codeflow.
While i like the idea, this needs a bit of design work before get merged! this would be the first event based API to it sets a precedence, also it global, dispatched in the document, which is not ideal... I agree this is a problem to solve though
Yes, normally I'd also prefer using a store, like the context used by useNavigate for example, and this is what sveltekit does for example https://kit.svelte.dev/docs/modules#$app-stores-navigating
But I couldn't make that work in qwik because I think of the DOM update buffering causing the DOM not to react in real time in that case until the navigation is finished. If there was a way to expose a store with real-time reactivity this would be an alternative.
Right now using global events seems to be the best way to implement this.
Also I think that using the events we could also make the navigationstart cancellable, so similar to the beforeunload event one could listen to that and on some pages ask user for confirmation before leaving and call prevent default. In that case qwik could abort the client side navigation.
I don't think this is as easily doable with a store?
Hi @jwickers 👋 Sorry for the silence on this PR 🙈 i really like the idea and i guess many devs coming from other frameworks are used to these events for showing loaders, etc.
@manucorporat : do you have smth on the radar/roadmap for this or a rough idea of how this could be implemented? also happy to give a hand for this feature ✋ 😄
@jwickers is this PR closed by #3244? cc @billykwok
I'm not using this PR as we did something else that works well enough using the current APIs, so yeah I'll close it.
Thanks 👍