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

[Bug] Modern template always sets the first navbar element as active #9877

Closed
MiTschMR opened this issue Apr 16, 2024 · 2 comments · Fixed by #10204
Closed

[Bug] Modern template always sets the first navbar element as active #9877

MiTschMR opened this issue Apr 16, 2024 · 2 comments · Fixed by #10204
Labels
bug A bug to fix template The stock site template

Comments

@MiTschMR
Copy link

Describe the bug
The modern template sets the first navbar element always as active. It seems to disregard parts of the path in the url to fully check if the path matches and only then set it active.

To Reproduce
Steps to reproduce the behavior:

  1. Add an index.md in the root directory and reference it
  2. Add subfolders with elements
  3. Add a toc.yml in the root directory only referencing the subfolder toc's

Expected behavior
The first element in the navbar (meaning a subfolder) should not be set to active.

Context (please complete the following information):

  • OS: Windows 11
  • Docfx version: 2.76.0

Additional context
The default theme does not have this issue.

docfx.json:

{
  "build": {
    "globalMetadata": {
      "_appFooter": "<span>Copyright © 2024 MiTschMR Studios<br>Generated by <strong>DocFX</strong></span>"
    },
    "content": [
      {
        "files": [
          "toc.yml",
          "index.md",
          "releases.md"
        ]
      },
      {
        "src": "authentication",
        "files": [
          "toc.yml",
          "*.md",
          "**/*.md"
        ],
        "dest": "authentication"
      },
      {
        "src": "polls",
        "files": [
          "toc.yml",
          "*.md",
          "**/*.md"
        ],
        "dest": "polls"
      },
      {
        "src": "rewards",
        "files": [
          "toc.yml",
          "*.md",
          "**/*.md"
        ],
        "dest": "rewards"
      },
      {
        "src": "streams",
        "files": [
          "toc.yml",
          "*.md",
          "**/*.md"
        ],
        "dest": "streams"
      },
      {
        "src": "users",
        "files": [
          "toc.yml",
          "*.md",
          "**/*.md"
        ],
        "dest": "users"
      }
    ],
    "resource": [
      {
        "files": [
          "resources/**/*",
          "logo.svg",
          "favicon.ico",
          "versions.json"
        ]
      }
    ],
    "template": [ "default", "modern", "templates/material" ],
    "dest": "_site"
  }
}

toc.yml (root):

- name: Authentication
  href: authentication/
- name: Rewards
  href: rewards/
- name: Streams
  href: streams/
- name: Users
  href: users/
- name: Releases
  href: ./releases.md

Website with modern template:
image

Website with default template (built with version 2.66.0):
https://mitschmr-studios.io/documentation/twitch/

@MiTschMR MiTschMR added the bug A bug to fix label Apr 16, 2024
@filzrev
Copy link
Contributor

filzrev commented Apr 22, 2024

Reported issue seems to be caused by following functions.

  • findActiveItem
  • commonUrlPrefix

function findActiveItem(items: (NavItem | NavItemContainer)[]): NavItem {
const url = new URL(window.location.href)
let activeItem: NavItem
let maxPrefix = 0
for (const item of items.map(i => 'items' in i ? i.items : i).flat()) {
if (isExternalHref(item.href)) {
continue
}
const prefix = commonUrlPrefix(url, item.href)
if (prefix > maxPrefix) {
maxPrefix = prefix
activeItem = item
}
}
return activeItem
}
function commonUrlPrefix(url: URL, base: URL): number {
const urlSegments = url.pathname.split('/')
const baseSegments = base.pathname.split('/')
let i = 0
while (i < urlSegments.length && i < baseSegments.length && urlSegments[i] === baseSegments[i]) {
i++
}
return i
}

pathname.split('/') generate string array with heading element is empty string.
So currently commonUrlPrefix always return value >=1.
And first element is selected as active item.

To resolve this issue. It need to exclude empty string from comparison.
And It might need following additional logics.

  • Support root (/) navigation item.
  • Support default active item to maintain compatibility with current behavior.
    (e.g. https://dotnet.github.io/docfx display Docs as active item when opening /index.html)

@yufeih yufeih added the template The stock site template label May 11, 2024
@MiTschMR
Copy link
Author

Any news on this? I would love to update to the modern template, but this bug prevents me from doing so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug to fix template The stock site template
Projects
None yet
3 participants