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

Visualized the text content of the "The Full Navigation Resolution Flow" section. #1486

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

niceplugin
Copy link
Contributor

Visualized the text content of the last section of the "guide/advanced/navigation-guards.md" document: "The Full Navigation Resolution Flow".

If there is no problem with the content, it seems easier to understand than the text.

If you decide to use this image, duplicate it on the vue team's figma.
The figma link annotations in the documentation also need to be corrected.

If there are any problems, please reject this PR (with comments).

…d/navigation-guards.md" document: "The Full Navigation Resolution Flow".
@netlify
Copy link

netlify bot commented Jul 24, 2022

Deploy Preview for vue-router ready!

Name Link
🔨 Latest commit fe91631
🔍 Latest deploy log https://app.netlify.com/sites/vue-router/deploys/62e020d7d870bb000848c2b1
😎 Deploy Preview https://deploy-preview-1486--vue-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

This is pretty neat! Do you have the editable file? We should remove the vue lifecycle hooks on the right (beforeUpdate and update) as there is no guarantee they will happen.
Also note beforeRouteLeave happens first and that beforeRouteEnter's next happen after DOM updates but there is no guarantee it's exactly after mounted or any other lifecycle hook

- Removed misleading Vue lifecycles
- More detailed router flows
@niceplugin
Copy link
Contributor Author

Image changed:

  • Removed misleading Vue lifecycles
  • More detailed router flows

Add content:

  • Example implemented in code sandbox

There are no editable files, but the images were made in Figma.
If duplicated this project with "your or vue team's" figma account, can edit it in figma.
This is an image link that can be duplicated:
https://www.figma.com/file/nxHzZVExlwnOiWhfeDU0nB/Vue-Router-Flow


Also note beforeRouteLeave happens first and that beforeRouteEnter's next happen after DOM updates but there is no guarantee it's exactly after mounted or any other lifecycle hook

This is likely to be understood by users through more detailed images and examples.
If I misunderstood technically and wrote the content, please reject it again(with comments).

@niceplugin niceplugin requested a review from posva July 25, 2022 15:06
Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

  • beforeRouteLeave happens at the very beginning, before beforeEach.
  • There isn't a isFailed scenario. It's a pipeline that any navigation guard can stop at any time: it doesn't branch. I think it's much easier to just keep one timeline / one vertical arrow
  • Between beforeResolve and afterEach the navigation is confirmed
  • The last part should really become DOM updates instead of setup/mounted/beforeUnmount because there is no guarantee in the order of these compared to the router
  • beforeRouteEnter next happens after that

- Flow modified
@niceplugin
Copy link
Contributor Author

Image changed:

  • Flow modified

PS.
I've seen you visit my figma project. Now you can also edit this.

@niceplugin niceplugin requested a review from posva July 26, 2022 17:19
@niceplugin
Copy link
Contributor Author

Has changed file any problems?
Looking forward to your feedback :)

@posva
Copy link
Member

posva commented Aug 8, 2022

Thanks a lot, I will get back to this later on once I update vue router docs.

@posva posva added the docs Concerns the documentation content or the documentation website label Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Concerns the documentation content or the documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants