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

fix(theme): fix ineffective activeMatch configuration #3355

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Red-Asuka
Copy link

In the settings of the default theme's Navbar, we can set the activeMatch for Navbar items with children. However, when the regular expression does not match and one of the links in the children matches the current page, it will be highlighted incorrectly.

This is the definition of NavItemWithChildren in the default theme:

  export interface NavItemWithChildren {
    text?: string
    items: (NavItemChildren | NavItemWithLink)[]

    /**
     * `activeMatch` is expected to be a regex string. We can't use actual
     * RegExp object here because it isn't serializable
     */
    activeMatch?: string
  }

@Red-Asuka
Copy link
Author

@brc-dd , could you please review the following PR? This PR fixes a bug where the theme navbar configuration does not respect the NavItem's activeMatch. I'll illustrate through a real-life example.

Let's say I have a dropdown list for versioning - I wanted to highlight only the specific version number and not the "Versions" tab. So I set an activeMatch that wouldn't practically be matched, but to no avail - "Versions" was still highlighted. I perceive this as a bug; The correct logic should foremost respect the parent menu's activeMatch.

const nav = {
  text: 'Versions',
  activeMatch: `^/no-active/`,
  items: [
    { text: 'v1.0', link: '/docs/v1' },
    { text: 'v2.0', link: '/docs/v2' },
    { text: 'v3.0', link: '/docs/v3' },
  ]
}
image

@github-actions github-actions bot removed the stale label Apr 12, 2024
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

2 participants