-
Notifications
You must be signed in to change notification settings - Fork 567
fix: normalize URL comparison in TOC highlighting #1463
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
base: master
Are you sure you want to change the base?
Conversation
…gested by reviewer
|
@GitToTheHub Thank you for the review! I've updated both files to use the cleaner single-line comparison as you suggested: Changed from: To: This is much cleaner and more readable. Changes have been pushed. Thanks for the improvement suggestion! |
|
Very good :) |
|
@erisu Is this fine to merge? |
|
Locally, the menu does not appear to be functioning as expected. Some elements seem duplicated, and the overall layout appears inconsistent. For example, the top set of links appears flattened and is not interactive. The lower section, which seems to mirror the top with some formatting and contain navigational links; however, all items are highlighted simultaneously. I am also not certain whether removing the “/” is the correct solution. Based on my testing, it does not remove only the trailing slash as described in the PR description, but instead removes all slashes. I also tried clearing previous builds, rebuilding from scratch, and clearing the cache to confirm that this is not an issue on my end. |
|
Thanks for verifying this, so @dheeraj12347 checked it not correctly? |
|
Thanks for the detailed reviews and for testing the menu behavior locally. You were right that using remove: '/' removed all slashes, not just a trailing one, and that it could cause the menu/highlighting issues you described. I’ve updated both www/_includes/toc_recursive_main.html and www/_includes/toc_recursive_dropdown.html so that entry_active compares include.my_entry and entry.url after only normalizing a possible trailing /, while leaving intermediate path segments unchanged. After a clean local rebuild, the TOC no longer appears duplicated/flattened, and only the correct entry is highlighted on the pages I tested. Please let me know if this approach looks good to you or if you’d prefer any further adjustments. |
Problem
Table of Contents (TOC) highlighting is not working correctly for some pages.
Root Cause
The URL comparison logic in
toc_recursive_main.htmlandtoc_recursive_dropdown.htmlwas failing due to trailing slash differences betweeninclude.my_entryandentry.url.Example:
/docs/api//docs/apiThese look the same but don't match as strings.
Solution
Normalize both URLs before comparison by removing trailing slashes using Jekyll's
removefilter.Added:
{% assign normalized_my_entry = include.my_entry | remove: '/' %} {% assign normalized_entry_url = entry.url | remove: '/' %} ## Solution Normalize both URLs before comparison by removing trailing slashes using Jekyll's `remove` filter. Added: ```liquid {% assign normalized_my_entry = include.my_entry | remove: '/' %} {% assign normalized_entry_url = entry.url | remove: '/' %} Then compare: text {% if normalized_my_entry == normalized_entry_url %} Files Changed www/_includes/toc_recursive_main.html www/_includes/toc_recursive_dropdown.html Why This Works Removes all / characters from both URLs before comparison Makes the comparison consistent regardless of trailing slashes Current page highlighting now works correctly No JavaScript changes needed