-
Notifications
You must be signed in to change notification settings - Fork 868
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
fix(modern-template): nav & toc active state breaks with external urls #9038
Conversation
310d471
to
d1dac25
Compare
d1dac25
to
56c621c
Compare
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #9038 +/- ##
=======================================
Coverage 77.56% 77.57%
=======================================
Files 591 591
Lines 24577 24577
=======================================
+ Hits 19064 19065 +1
+ Misses 5513 5512 -1 ☔ View full report in Codecov by Sentry. |
@yufeih i'm unsure about how to specially add a test for the "active" variable, but I can't spend more time to figure it out. I hope that pr is good to go, otherwise I'd be happy if someone could fix it. |
548659c
to
ab70080
Compare
btw what's up with I noticed it since upgrading docfx Sorry for the ci spam also, I tried fixing the snapshot ci. Doesn't work tho due to limited workflow permissions. |
ab70080
to
378967f
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.
Thank you @Lulalaby !
dotnet#9038) * fix: nav & toc active state breaks with external urls * chore: move check to helper & add tests
The current problem
When having an external url defined in the toc.yml, the active state toggler sets all external links & the current page as active
Screenshots
Proposed Solution
By checking whether the items
hostname
orport
differs fromwindow.location
, we can set the active state correctly.Screenshots