feat: nav dropdown, mobile drawer, phosphor icons, dark mode fix, responsive scale#846
Conversation
…ponsive scale - Replace all icons with Phosphor Icons (CDN CSS, ph-* class approach) - Fix dark mode nav: header now uses dark semi-transparent bg in dark theme - Add desktop Browse hover dropdown with 8 categories in a 4-column grid - Add mobile full-height slide-out drawer from right with all nav items - Add responsive scale overrides for spacing/font sizes at mobile breakpoints - Add color-scheme per theme to prevent OS-level dark UI glitches - Update CATEGORIES icon names to valid Phosphor icon identifiers https://claude.ai/code/session_017G3gEzdfRafmqFno5dfWQH
|
Warning Review limit reached
More reviews will be available in 9 minutes and 24 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ 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 |
|
Missing required section(s): Linked issues, Changelog, Global DoD checklist This is a post-merge backstop for admin bypasses. Please review branch protection for develop. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4966f4d52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| .ag-browse-panel { | ||
| position: absolute; | ||
| top: calc(100% + 8px); |
There was a problem hiding this comment.
On desktop pointer devices, the Browse panel is opened by mouseenter but closed on mouseleave of the wrapper; positioning the panel at top: calc(100% + 8px) creates an 8px dead zone between the trigger and the panel. Moving the cursor from the button toward any category crosses that gap, fires mouseleave, and hides the menu before the links can be reached, so the hover dropdown is effectively unusable without keyboard/touch-specific interaction.
Useful? React with 👍 / 👎.
| width: 100%; | ||
| height: 100dvh; | ||
| z-index: 200; |
There was a problem hiding this comment.
At the mobile breakpoint, the drawer opens as a full-viewport fixed element while the overlay is behind it and the hamburger/close button remains in the nav underneath; because the drawer appears later in the DOM at the same z-index, it covers the changed “X” control. Touch or mouse users who open the drawer and then choose not to navigate have no visible pointer target to close it, so the drawer can only be dismissed via Escape or by following a link.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request refactors the navigation header to introduce a sticky navigation bar featuring a desktop 'Browse' mega-dropdown, a mobile full-screen drawer, and a backdrop overlay. It also integrates Phosphor Icons, minifies the theme initialization script to prevent theme flashing, and implements event delegation for theme toggling. The review feedback highlights several critical accessibility and usability improvements: resolving a z-index conflict that traps mobile users in the drawer, refactoring the mobile scroll-lock to use CSS classes instead of direct inline styles, restoring dynamic aria-label updates for the hamburger button, and converting the dropdown menu to a Disclosure pattern to comply with ARIA guidelines.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| height: 72px; | ||
| padding: 0 var(--gutter); | ||
| background: rgba(255, 255, 255, 0.97); | ||
| z-index: 200; |
There was a problem hiding this comment.
Both .ag-nav and .ag-mobile-drawer are configured with z-index: 200. Because the drawer is defined later in the DOM, it will render on top of the header, completely covering the hamburger/close button. Since there is no close button inside the drawer and the overlay is also covered (due to width: 100%), mobile users will be trapped in the drawer with no way to close it. Increasing the header's z-index to 210 ensures it stays on top of the drawer, keeping the close button accessible.
z-index: 210;
| const openDrawer = () => { | ||
| hamburger.setAttribute("aria-expanded", "true"); | ||
| drawer.setAttribute("aria-hidden", "false"); | ||
| overlay.setAttribute("aria-hidden", "false"); | ||
| document.body.style.overflow = "hidden"; | ||
| }; | ||
| const closeDrawer = () => { | ||
| hamburger.setAttribute("aria-expanded", "false"); | ||
| drawer.setAttribute("aria-hidden", "true"); | ||
| overlay.setAttribute("aria-hidden", "true"); | ||
| document.body.style.overflow = ""; | ||
| }; |
There was a problem hiding this comment.
This block has two issues:
- Setting
document.body.style.overflow = "hidden"directly in JS causes the page to remain unscrollable if the user resizes the window to desktop width while the drawer is open. Toggling a class ondocument.bodyand handling the overflow lock in CSS inside the mobile media query is much more robust. - The hamburger button's
aria-labelis statically set to"Open navigation menu", which is a regression from the original code that dynamically updated it to"Close navigation menu"when open.
const openDrawer = () => {
hamburger.setAttribute("aria-expanded", "true");
hamburger.setAttribute("aria-label", "Close navigation menu");
drawer.setAttribute("aria-hidden", "false");
overlay.setAttribute("aria-hidden", "false");
document.body.classList.add("ag-drawer-open");
};
const closeDrawer = () => {
hamburger.setAttribute("aria-expanded", "false");
hamburger.setAttribute("aria-label", "Open navigation menu");
drawer.setAttribute("aria-hidden", "true");
overlay.setAttribute("aria-hidden", "true");
document.body.classList.remove("ag-drawer-open");
};
| @media (max-width: 960px) { | ||
| .ag-nav-desktop { display: none; } | ||
| .ag-hamburger-btn { display: flex; } | ||
| } |
There was a problem hiding this comment.
Add the :global(body.ag-drawer-open) rule inside the mobile media query to lock scrolling only on mobile viewports when the drawer is open.
@media (max-width: 960px) {
.ag-nav-desktop { display: none; }
.ag-hamburger-btn { display: flex; }
:global(body.ag-drawer-open) {
overflow: hidden;
}
}
| <button | ||
| class="ag-nav-link ag-browse-trigger" | ||
| aria-expanded="false" | ||
| aria-haspopup="menu" | ||
| aria-controls="ag-browse-panel" | ||
| > |
There was a problem hiding this comment.
Using role="menu" and role="menuitem" without implementing keyboard arrow navigation violates ARIA guidelines and confuses screen reader users. Since this is a simple list of navigation links, it is highly recommended to use the Disclosure pattern instead by removing aria-haspopup="menu", role="menu", and role="menuitem".
<button
class="ag-nav-link ag-browse-trigger"
aria-expanded="false"
aria-controls="ag-browse-panel"
>
| <div | ||
| class="ag-browse-panel" | ||
| id="ag-browse-panel" | ||
| role="menu" | ||
| aria-label="Browse categories" | ||
| > |
There was a problem hiding this comment.
Part of the Disclosure pattern conversion. Remove role="menu" and aria-label from the panel container.
<div
class="ag-browse-panel"
id="ag-browse-panel"
>
| <a | ||
| href={`${base}c/${cat.id}/`} | ||
| class="ag-browse-card" | ||
| role="menuitem" | ||
| > |
There was a problem hiding this comment.
Part of the Disclosure pattern conversion. Remove role="menuitem" from the category links.
<a
href={`${base}c/${cat.id}/`}
class="ag-browse-card"
>
Signed-off-by: Ash Shaw <ashley@lightspeedwp.agency>
🔍 Reviewer Summary for PR #846CI Status: ✅ Recommendations
|
There was a problem hiding this comment.
Pull request overview
This PR updates the “Awesome GitHub” website UI to introduce a new navigation experience (desktop browse dropdown + mobile drawer), improve responsive spacing/type scaling, and refine theme handling (including color-scheme hints and updated icon usage).
Changes:
- Adds responsive CSS variable overrides (gutters, section spacing, heading/lead scale) and sets
color-schemeper theme to reduce OS/UI theme glitches. - Updates catalogue category icon identifiers to match Phosphor icon naming.
- Refactors layout/theme initialisation and introduces Phosphor web icon stylesheet; nav is reworked for desktop browse dropdown and a mobile right-side drawer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
website/src/styles/awesome-github.css |
Adds breakpoint-based variable overrides and color-scheme rules per theme. |
website/src/lib/catalogue.ts |
Updates category icon names for Phosphor compatibility. |
website/src/layouts/AwesomeGithubLayout.astro |
Adds early theme init script changes and includes Phosphor web CSS; adjusts main layout sizing + skip link styling. |
website/src/components/AwesomeGithub/AwesomeGithubNav.astro |
Large nav refactor: desktop “Browse” panel, mobile drawer/overlay, new icon markup, and associated JS/CSS changes. |
Comments suppressed due to low confidence (1)
website/src/components/AwesomeGithub/AwesomeGithubNav.astro:125
- There is a leftover template fragment from the previous nav (
{link.label}/</a>/</nav>) immediately after the new.ag-nav-actions. This leaves the<header>/.ag-nav-innerstructure unbalanced and will break Astro compilation/rendering. Close the new.ag-nav-inner+<header>here, and remove the legacy nav markup that follows (lines 127–160).
{link.label}
</a>
))}
</nav>
| class="ag-nav-link ag-browse-trigger" | ||
| aria-expanded="false" | ||
| aria-haspopup="menu" | ||
| aria-controls="ag-browse-panel" | ||
| > |
| class="ag-browse-panel" | ||
| id="ag-browse-panel" | ||
| role="menu" | ||
| aria-label="Browse categories" | ||
| > |
| <a | ||
| href={`${base}c/${cat.id}/`} | ||
| class="ag-browse-card" | ||
| role="menuitem" | ||
| > |
| </div> | ||
| </header> |
| hamburger.addEventListener("click", () => { | ||
| hamburger.getAttribute("aria-expanded") === "true" ? closeDrawer() : openDrawer(); | ||
| if (hamburger && menu) { | ||
| const closeMenu = () => { | ||
| hamburger.setAttribute("aria-expanded", "false"); |
| overlay.addEventListener("click", closeDrawer); | ||
|
|
||
| drawer.querySelectorAll("a").forEach((link) => { | ||
| link.addEventListener("click", closeDrawer); | ||
| // Close on link click (mobile) |
| .ag-mobile-overlay[aria-hidden="false"] { | ||
| opacity: 1; | ||
| visibility: visible; | ||
| .theme-icon-light, | ||
| .theme-icon-dark { |
| @media (max-width: 480px) { | ||
| .ag-brand-text { display: none; } | ||
| .ag-nav-hamburger[aria-expanded="true"] .hamburger-line:nth-child(2) { |
| <!-- Phosphor Icons (regular weight) --> | ||
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@phosphor-icons/web@2.1.1/src/regular/style.css" /> |
https://claude.ai/code/session_017G3gEzdfRafmqFno5dfWQH