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.

Sign up for GitHub

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

Jump to bottom

docs: add navigation failures #3220

Merged
merged 17 commits into from
Jul 31, 2020
Merged

docs: add navigation failures #3220

merged 17 commits into from
Jul 31, 2020

Conversation

posva
Copy link
Member

@posva posva commented Jun 5, 2020 •
edited
Loading

Based on vuejs/rfcs#150

@posva posva added the docs label Jun 5, 2020
@posva posva mentioned this pull request Jun 16, 2020
Merged
shentao
shentao approved these changes Jun 24, 2020
View reviewed changes
Copy link
Member

@shentao shentao left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor comments.

docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
bencodezen
bencodezen reviewed Jun 24, 2020
View reviewed changes
Copy link
Member

@bencodezen bencodezen left a comment

Choose a reason for hiding this comment

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

Added some suggestions for rephrasing in order to improve clarity and maintain some consistency and parallelism throughout the doc.

docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
docs/guide/advanced/navigation-failures.md Outdated Show resolved Hide resolved
@posva posva mentioned this pull request Jul 20, 2020
Closed
posva and others added 17 commits July 31, 2020 21:14
@posva
docs: add navigation failures
e24c57f
@posva @NataliaTepluhina
Update docs/guide/advanced/navigation-failures.md
e69d78b 
Co-authored-by: Natalia Tepluhina <[email protected]>
@posva
Update docs/guide/advanced/navigation-failures.md
a554157
@posva
feat(errors): NavigationDuplicated name for backwards compatibility
02f7eaa
@posva
docs: use singular form
a93434f
@posva
feat(errors): expose isRouterError
cea40b2
@posva
refactor: fix flow types
c567ef5
@posva
types(errors): add TS types
c47f9c4
@posva
docs: add new in comment [skip ci]
bb90585
@posva
docs: rename isRouterError [skip ci]
902c82d
@posva
docs: remove extra if [skip ci]
e532401
@posva
docs: split background story and explanation [skip ci]
5e03c90
@posva @bencodezen
Apply suggestions from code review
4492bbb 
Co-authored-by: Ben Hong <[email protected]>
@posva
refactor: adapt values for errors
73c1d58
@posva @bencodezen
Apply suggestions from code review
7ef00d1 
Co-authored-by: Ben Hong <[email protected]>
@posva
refactor: isRouterError -> isNavigationFailure
7fd4b93
@posva
refactor: add missing type rename [skip ci]
d8ce259
@posva posva merged commit dfc2892 into dev Jul 31, 2020
@posva posva deleted the docs/navigation-failures branch July 31, 2020 19:14
@posva
Copy link
Member Author

posva commented Jul 31, 2020

Thanks a lot @NataliaTepluhina @shentao, @trygveaa and @bencodezen for the feedback!

@trygveaa
Copy link

trygveaa commented Sep 4, 2020

@posva: Hm, I tried import { NavigationFailureType, isNavigationFailure } from 'vue-router' in typescript, but it says they're not exported. Looking at the types, it seems that they're only static types on the VueRouter class. Is that correct?

@posva
Copy link
Member Author

posva commented Sep 4, 2020

Fixed at ecc8e27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bencodezen bencodezen bencodezen left review comments

@shentao shentao shentao approved these changes

@trygveaa trygveaa trygveaa left review comments

@NataliaTepluhina NataliaTepluhina NataliaTepluhina approved these changes

Assignees
No one assigned
Labels
docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@posva @trygveaa @shentao @bencodezen @NataliaTepluhina