fix: correct invalid grid nesting, heading nesting, and navbar collapse - #183
Merged
Conversation
Three markup issues found while auditing the site: - index.astro nested a `.row` directly inside another `.row` in the Sponsors and Collaborators sections. The inner row's -15px negative margins applied with no `.col` padding to cancel them. Split each into two sibling rows, moving `mb-4` to the logo row so spacing before the following `<hr>` is unchanged. - blog.astro wrapped an `<h2>` in an `<a>`. Inverted to put the anchor inside the heading. The `<a>` still supplies the link color the `<h2>` was inheriting, so it renders identically. - The navbar `<ul>` was not inside a `.collapse.navbar-collapse` and had no toggler, so `navbar-expand-md` just stacked nine links vertically on mobile. Added the collapse wrapper and a `.navbar-toggler`. Bootstrap's collapse JS needs jQuery, which the site does not load, and `.collapse:not(.show)` would otherwise hide the nav outright below the md breakpoint. So the toggler ships a small script that toggles `.show` and keeps `aria-expanded` in sync, following the same no-jQuery approach the Carousel component already uses. Above md, `.navbar-collapse` is `display: flex !important` and the toggler is `display: none`, so the desktop nav is untouched and a stale `.show` is harmless when resizing back up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three markup issues found while auditing the site.
.rownested directly inside.row—src/pages/index.astroThe Sponsors and Collaborators sections put the heading and the logo grid as siblings inside a single
.row, so the inner.row's -15px negative margins applied with no.colpadding to cancel them. Split each into two sibling rows, movingmb-4to the logo row so the spacing before the following<hr>is unchanged.Verified in the build output:
<div>open/close counts balance (216/216) and there are no remaining row-in-row occurrences.Heading wrapped in an anchor —
src/pages/blog.astro<a><h2>…</h2></a>→<h2><a href="…">…</a></h2>. Renders identically; the<a>still supplies the link color the<h2>was inheriting.Navbar with no collapse or toggler —
src/layouts/Layout.astroThe
<ul>was not inside a.collapse.navbar-collapseand there was no toggler, sonavbar-expand-mdjust stacked nine links vertically on mobile with no hamburger.This one needed more than markup. Bootstrap's
.collapse:not(.show){display:none}would have hidden the nav outright below 768px, and Bootstrap 4's collapse JS requires jQuery, which this site doesn't load. So the toggler ships a small script that toggles.showand keepsaria-expandedin sync — the same no-jQuery approach theCarouselcomponent already uses. The button getsaria-labelandaria-controlssince it's a new control.Checked against the compiled Bootstrap CSS that both
.navbar-expand-md .navbar-collapse{display:flex!important}and.navbar-expand-md .navbar-toggler{display:none}sit inside@media (min-width:768px), so the desktop nav is untouched and a leftover.showis harmless when resizing back up..navbar-dark .navbar-toggler-iconsupplies a white hamburger, so the button is visible on thebg-infobar.Behavior change
Below 768px the nine nav links previously stacked vertically and were always visible; they are now behind the hamburger. That's the point of the fix, but it is the only user-visible change in this PR — worth a look on a phone before merging.
Testing
astro check— 0 errors, 0 warnings, 0 hintsastro build— 10 pages builtNo accessibility, SEO, performance, or security items from the wider audit are touched here.
🤖 Generated with Claude Code