fix: logo now redirects to main circuitverse.org domain (#6437)#264
fix: logo now redirects to main circuitverse.org domain (#6437)#264vivekyadav-3 wants to merge 1 commit intoCircuitVerse:masterfrom
Conversation
| </button> | ||
|
|
||
| <div class="collapse navbar-collapse text-center" id="navigation"> | ||
| <ul class="navbar-nav ml-auto"> |
There was a problem hiding this comment.
- contains direct text content. Wrap in
- .
- and
- , <script>, or as direct children.
Details
Screen readers announce list structure ('list with 5 items') based on proper markup. Placing non-
- elements directly inside
- or
- elements, or if you need wrapper divs for styling, restructure your CSS to style the
- elements directly.
- breaks this structure. Wrap content in
- elements directly inside
- must only contain
- , <script>, or as direct children.
| </ul> | ||
| {{ if .Site.Params.search.enable }} {{ "<!-- search -->" | safeHTML }} | ||
| <div class="search"> | ||
| <button id="searchOpen" class="search-btn"> |
There was a problem hiding this comment.
WCAG 4.1.2: Button has no discernible text.
Buttons must have discernible text.
Details
Classes: search-btn
Screen reader users need to know what a button does. Add visible text content, aria-label, or aria-labelledby. For icon buttons, use aria-label describing the action (e.g., aria-label='Close'). If the button contains an image, ensure the image has alt text describing the button's action.
Also on line 69.
| placeholder="Type & Hit Enter..." | ||
| /> | ||
| </form> | ||
| <button id="searchClose" class="search-close"> |
There was a problem hiding this comment.
WCAG 4.1.2: Button has no discernible text.
WalkthroughA new Hugo layout partial is added to implement a fixed-top responsive navigation bar component. The navigation includes a brand logo, mobile hamburger menu toggle, and a collapsible main menu with dropdown support for nested items. It conditionally renders a search interface when enabled in site parameters. The component uses Hugo templating to dynamically populate navigation items and properly resolves URLs. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes the blog logo link to redirect users to the main CircuitVerse domain instead of the blog home page. The change addresses user confusion by creating a Hugo theme override that explicitly sets the logo href to https://circuitverse.org/ while keeping the "Home" navigation link pointing to the blog home.
Changes:
- Override the northendlab-hugo theme's header partial to customize the logo link behavior
- Update logo href from
.Site.BaseURL(blog home) to hardcoded main site URL
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class="container"> | ||
| <!-- navbar --> | ||
| <nav class="navbar px-0 navbar-expand-lg navbar-light bg-transparent"> | ||
| <a class="navbar-brand" href="https://circuitverse.org/"> |
There was a problem hiding this comment.
The URL is hardcoded, which makes the template less flexible and harder to maintain. Consider making this configurable by adding a parameter to config.toml (e.g., params.main_site_url or params.logo_url) and then reference it here with {{ .Site.Params.main_site_url }}. This would allow for easier configuration changes without modifying template files, which is a Hugo best practice.
| <a class="navbar-brand" href="https://circuitverse.org/"> | |
| <a class="navbar-brand" href="{{ .Site.Params.main_site_url | default "https://circuitverse.org/" }}"> |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
layouts/partials/header.html (1)
5-5: Logo redirect fix looks correct; consider a site param for future maintainability.The hardcoded
https://circuitverse.org/achieves the PR objective. As an optional improvement, externalising this to a site param (e.g.,mainSiteURL) would let you change or override the target per environment without modifying template code.♻️ Suggested optional refactor
In
config.toml/hugo.toml:[params] mainSiteURL = "https://circuitverse.org/"In the template:
- <a class="navbar-brand" href="https://circuitverse.org/"> + <a class="navbar-brand" href="{{ .Site.Params.mainSiteURL | default `https://circuitverse.org/` }}">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouts/partials/header.html` at line 5, The anchor in layouts/partials/header.html currently hardcodes the main site URL in the <a class="navbar-brand"> tag; refactor it to use a Hugo site param (e.g., .Site.Params.mainSiteURL) so the target can be configured per environment. Update the template to read the param with a sensible fallback (like "https://circuitverse.org/") when .Site.Params.mainSiteURL is unset, and add the parameter to config.toml/toml equivalent under [params] as mainSiteURL to allow overrides without touching template code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@layouts/partials/header.html`:
- Around line 56-71: Add accessible names to the icon-only buttons by adding
descriptive aria-label attributes to the elements with id "searchOpen" and
"searchClose" (e.g., aria-label="Open search" and aria-label="Close search") and
replace the unescaped ampersand in the input with an HTML entity (change the
placeholder on element id "search-query" from "Type & Hit Enter..." to use
"&") so the buttons are accessible and the HTML validates.
- Around line 12-19: The hamburger toggle button (the element with class
"navbar-toggler" that targets "#navigation") is missing accessibility
attributes; update the button to include aria-label="Toggle navigation",
aria-controls="navigation", and an initial aria-expanded="false" so screen
readers announce its purpose and controlled region, and ensure the collapse
behavior (Bootstrap/JS that manages the "#navigation" collapse) updates
aria-expanded as the menu opens and closes.
---
Nitpick comments:
In `@layouts/partials/header.html`:
- Line 5: The anchor in layouts/partials/header.html currently hardcodes the
main site URL in the <a class="navbar-brand"> tag; refactor it to use a Hugo
site param (e.g., .Site.Params.mainSiteURL) so the target can be configured per
environment. Update the template to read the param with a sensible fallback
(like "https://circuitverse.org/") when .Site.Params.mainSiteURL is unset, and
add the parameter to config.toml/toml equivalent under [params] as mainSiteURL
to allow overrides without touching template code.
| <button | ||
| class="navbar-toggler border-0 px-0" | ||
| type="button" | ||
| data-toggle="collapse" | ||
| data-target="#navigation" | ||
| > | ||
| <i class="ti-menu text-dark h3"></i> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for Bootstrap version in configuration files
fd -e json -e toml -e yaml -e mod -e lock -e txt | head -20Repository: CircuitVerse/Blog
Length of output: 85
🏁 Script executed:
# Search for Bootstrap version references in common config files
find . -maxdepth 3 \( -name "package.json" -o -name "go.mod" -o -name "config.toml" -o -name "config.yaml" -o -name "theme.toml" \) -type f -exec cat {} \; 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 3242
🏁 Script executed:
# Check for data-toggle vs data-bs-toggle usage in the codebase
rg 'data-toggle|data-bs-toggle' --type html -cRepository: CircuitVerse/Blog
Length of output: 91
🏁 Script executed:
# Search for Bootstrap imports or version indicators
rg 'bootstrap' --type json --type toml --type yaml -A 2 -B 2Repository: CircuitVerse/Blog
Length of output: 499
🏁 Script executed:
# Check Bootstrap version from actual plugin files
fd -type f -path "*/plugins/bootstrap/*" | head -20Repository: CircuitVerse/Blog
Length of output: 231
🏁 Script executed:
# Try to find version info in Bootstrap files
find . -path "*/plugins/bootstrap/*" -type f \( -name "*.css" -o -name "*.js" \) | head -5 | xargs head -20 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Check the header.html file to see full context and all data-toggle usages
cat -n layouts/partials/header.html | head -40Repository: CircuitVerse/Blog
Length of output: 1644
🏁 Script executed:
# Check Bootstrap version from the CSS file header or first lines
head -100 plugins/bootstrap/bootstrap.min.css 2>/dev/null | grep -o "Bootstrap v[0-9.]*" | head -1Repository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Alternative: check for Bootstrap version in CSS or JS comments
find . -path "*/plugins/bootstrap/*" \( -name "*.css" -o -name "*.js" \) -exec head -5 {} + 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Search for version strings in package dependencies or lockfiles
find . -maxdepth 3 -type f \( -name "package-lock.json" -o -name "yarn.lock" -o -name ".versions" \) 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Try to find Bootstrap files with a different approach
find . -name "bootstrap.min.css" -o -name "bootstrap.min.js" 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Check if files exist and get their content
ls -lh plugins/bootstrap/ 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Search for any README or version files
find plugins/bootstrap/ -type f 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# List all files in the repository to understand structure
git ls-files | head -40Repository: CircuitVerse/Blog
Length of output: 1225
🏁 Script executed:
# Search for any minified bootstrap files anywhere
find . -name "*bootstrap*" 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Check if there's a theme subdirectory with Bootstrap
find . -path "*northendlab*" -o -path "*theme*" 2>/dev/null | head -20Repository: CircuitVerse/Blog
Length of output: 177
🏁 Script executed:
# Check Bootstrap files in the theme directory
find themes/northendlab-hugo -name "*bootstrap*" -type f 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Check the theme's config or README for Bootstrap version info
ls -la themes/northendlab-hugo/ 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 167
🏁 Script executed:
# Look at the header.html in the theme directly
cat themes/northendlab-hugo/layouts/partials/header.html | head -50Repository: CircuitVerse/Blog
Length of output: 145
🏁 Script executed:
# Check for Bootstrap version in the theme's plugins or static files
find themes/northendlab-hugo -type f -name "*.css" -o -name "*.js" | grep -i bootstrap | head -10Repository: CircuitVerse/Blog
Length of output: 43
🏁 Script executed:
# Check .gitmodules to see if theme is a git submodule
cat .gitmodules 2>/dev/nullRepository: CircuitVerse/Blog
Length of output: 185
🏁 Script executed:
# Check the theme directory status
git ls-files | grep themes/Repository: CircuitVerse/Blog
Length of output: 84
🏁 Script executed:
# The layouts directory is at the root, not in theme. Let's verify the structure
ls -la layouts/partials/ | head -20Repository: CircuitVerse/Blog
Length of output: 232
🏁 Script executed:
# Check if there are any bootstrap files in static or assets
find . -name "*bootstrap*" -type f 2>/dev/null | head -20Repository: CircuitVerse/Blog
Length of output: 43
🌐 Web query:
northendlab-hugo theme Bootstrap version
💡 Result:
The NorthendLab Hugo theme uses Bootstrap v4.5.0 (noted in the theme’s changelog for v1.0.0, released March 7, 2021). [1]
Sources:
[1] (gethugothemes.com)
Citations:
Add accessibility attributes to hamburger toggle button.
The icon-only button lacks aria-label, aria-controls, and aria-expanded attributes. Screen readers will not announce its purpose or state.
Suggested fix
<button
class="navbar-toggler border-0 px-0"
type="button"
data-toggle="collapse"
data-target="#navigation"
+ aria-label="Toggle navigation"
+ aria-controls="navigation"
+ aria-expanded="false"
>
<i class="ti-menu text-dark h3"></i>
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| class="navbar-toggler border-0 px-0" | |
| type="button" | |
| data-toggle="collapse" | |
| data-target="#navigation" | |
| > | |
| <i class="ti-menu text-dark h3"></i> | |
| </button> | |
| <button | |
| class="navbar-toggler border-0 px-0" | |
| type="button" | |
| data-toggle="collapse" | |
| data-target="#navigation" | |
| aria-label="Toggle navigation" | |
| aria-controls="navigation" | |
| aria-expanded="false" | |
| > | |
| <i class="ti-menu text-dark h3"></i> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@layouts/partials/header.html` around lines 12 - 19, The hamburger toggle
button (the element with class "navbar-toggler" that targets "#navigation") is
missing accessibility attributes; update the button to include
aria-label="Toggle navigation", aria-controls="navigation", and an initial
aria-expanded="false" so screen readers announce its purpose and controlled
region, and ensure the collapse behavior (Bootstrap/JS that manages the
"#navigation" collapse) updates aria-expanded as the menu opens and closes.
| <button id="searchOpen" class="search-btn"> | ||
| <i class="ti-search"></i> | ||
| </button> | ||
| <div class="search-wrapper"> | ||
| <form action="{{ `search` | absURL }}" class="h-100"> | ||
| <input | ||
| class="search-box px-4" | ||
| id="search-query" | ||
| name="s" | ||
| type="search" | ||
| placeholder="Type & Hit Enter..." | ||
| /> | ||
| </form> | ||
| <button id="searchClose" class="search-close"> | ||
| <i class="ti-close text-dark"></i> | ||
| </button> |
There was a problem hiding this comment.
Icon-only search buttons are missing aria-label; unescaped & in placeholder.
Two issues:
- Both
#searchOpenand#searchClosebuttons are icon-only with no accessible name, identical to the hamburger problem above. - Line 66:
placeholder="Type & Hit Enter..."— the bare&is technically invalid HTML (should be&). Browsers tolerate it, but it will fail HTML validation.
🛡️ Proposed fix
<button id="searchOpen" class="search-btn">
- <i class="ti-search"></i>
+ <i class="ti-search" aria-hidden="true"></i>
+ <span class="sr-only">Open search</span>
</button> <input
class="search-box px-4"
id="search-query"
name="s"
type="search"
- placeholder="Type & Hit Enter..."
+ placeholder="Type & Hit Enter..."
/> <button id="searchClose" class="search-close">
- <i class="ti-close text-dark"></i>
+ <i class="ti-close text-dark" aria-hidden="true"></i>
+ <span class="sr-only">Close search</span>
</button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@layouts/partials/header.html` around lines 56 - 71, Add accessible names to
the icon-only buttons by adding descriptive aria-label attributes to the
elements with id "searchOpen" and "searchClose" (e.g., aria-label="Open search"
and aria-label="Close search") and replace the unescaped ampersand in the input
with an HTML entity (change the placeholder on element id "search-query" from
"Type & Hit Enter..." to use "&") so the buttons are accessible and the HTML
validates.
The blog logo was previously linking to
.Site.BaseURL(the blog home), which confused users who expected it to take them back tocircuitverse.org.This PR overrides the theme header to explicitly set the logo
hreftohttps://circuitverse.org/, fixing issue #6437.Summary by CodeRabbit