-
Notifications
You must be signed in to change notification settings - Fork 381
Description
In Quarto website and probably book, when there is navbar for a page with a sidebar, the active nav is not the right one.
See below for Quarto Web - the CSS for active link being blue shows the overview is always selected
This was reported by @AshleyHenry15
This was tricky to debug, but this is because:
We do set the active nav based on some sidebar finding and href comparaison
quarto-cli/src/project/types/website/website-navigation.ts
Lines 555 to 572 in 13fd02c
// latch active nav link | |
const navLinks = doc.querySelectorAll("a.nav-link"); | |
for (let i = 0; i < navLinks.length; i++) { | |
const navLink = navLinks[i] as Element; | |
const navLinkHref = navLink.getAttribute("href"); | |
const sidebarLink = doc.querySelector( | |
'.sidebar-navigation a[href="' + navLinkHref + '"]', | |
); | |
// if the link is either for the current window href or appears on the | |
// sidebar then set it to active | |
if (sidebarLink || (navLinkHref === href)) { | |
navLink.classList.add("active"); | |
navLink.setAttribute("aria-current", "page"); | |
// terminate (only one nav link should be active) | |
break; | |
} | |
} |
but this has been broken by the fact the sidebar may have a logo with a link:
quarto-cli/src/resources/projects/website/templates/sidebar.ejs
Lines 35 to 46 in 13fd02c
<% if (sidebar.logo || (sidebar.title && !navbar)) { %> | |
<div class="pt-lg-2 mt-2 <%= alignCss %> sidebar-header<%= sidebar.logo && sidebar.title ? ' sidebar-header-stacked' : '' %>"> | |
<% if (sidebar.logo) { %> | |
<a href="<%- sidebar['logo-href'] || '/index.html' %>" class="sidebar-logo-link"> | |
<% if (sidebar.logo.light) { %> | |
<img src="<%- sidebar.logo.light.path %>" alt="<%- sidebar.logo.light.alt || '' %>" class="sidebar-logo light-content py-0 d-lg-inline d-none"/> | |
<% } %> | |
<% if (sidebar.logo.dark) { %> | |
<img src="<%- sidebar.logo.dark.path %>" alt="<%- sidebar.logo.dark.alt || '' %>" class="sidebar-logo dark-content py-0 d-lg-inline d-none"/> | |
<% } %> | |
</a> | |
<% } %> |
This part of the template is activated now always because sidebar.logo has been normalized in
- feature: light and dark logo #12996
and it is no more undefined
{
light: undefined,
dark: undefined,
}
So if(sidebar.logo)
is always true and this creates some HTML that are not expected in the page
<!-- sidebar -->
<nav id="quarto-sidebar" class="sidebar collapse collapse-horizontal quarto-sidebar-collapse-item sidebar-navigation floating overflow-auto">
<div class="pt-lg-2 mt-2 text-left sidebar-header sidebar-header-stacked">
<a href="./index.html" class="sidebar-logo-link">
</a>
</div>
which messes up with the search
quarto-cli/src/project/types/website/website-navigation.ts
Lines 561 to 563 in 13fd02c
const sidebarLink = doc.querySelector( | |
'.sidebar-navigation a[href="' + navLinkHref + '"]', | |
); |
hence the .active
class not being set on the right nav.
So
- We can't test
if(sidebar.logo)
anymore, as it seems it won't be undefined butsidebar.logo.light
andsidebar.logo.dark
will - We probably need to protect the search for avoid selecting the link inside the logo part.
This is a regression and we'll need to backport.