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

Update isActive logic to always checking equality #215

Merged
merged 1 commit into from Apr 23, 2018

Conversation

onevcat
Copy link
Contributor

@onevcat onevcat commented Apr 23, 2018

Current isActive checking will fail when a path is prefixed by another. It causes a problem when showing active indicator in the side bar.

eg:

Say we have two pages under api folder, which are uniwebview.md and uniwebviewmessage.md. When accessing api/uniwebviewmessage.html, the result when determining side bar active state for uniwebview page is:

routePath: "/api/uniwebviewmessage"
pagePath: "/api/uniwebview"

This leads to both uniwebviewmessage and uniwebview be rendered as actived, which is not correct:

snip20180423_1

By changing it to ===, it goes fine. However, I am not sure whether there would be a regression on this change or not, since there seems like lack of tests for this project.

@ulivz ulivz added help wanted Extra attention is needed semver: minor and removed help wanted Extra attention is needed labels Apr 23, 2018
@ycmjason
Copy link
Contributor

Not so sure why originally it checks equality only when either of them ends with / and check prefix otherwise. But your code looks alright to me, I have tested with my own site and it works ok.

@yyx990803 yyx990803 merged commit 9c93d8f into vuejs:master Apr 23, 2018
@yyx990803
Copy link
Member

Thanks, originally this function was used for isActive check for the header nav links as well, so it has as some left-over logic 😅

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

Successfully merging this pull request may close these issues.

None yet

4 participants