🐛 Fixed mobile admin nav bar reloading admin and its dark-mode active state#28609
Conversation
no ref - the mobile nav bar's active/hover state hardcoded bg-gray-200, which doesn't adapt to the theme, so the active item rendered as a bright light-gray pill on the dark bar in dark mode - use the theme-aware sidebar tokens (bg-sidebar-accent / text-sidebar-accent-foreground) instead, matching shade's canonical sidebar button styling
WalkthroughThe 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx build @tryghost/sodo-search |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/portal |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/admin-toolbar |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/activitypub |
✅ Succeeded | 2s | View ↗ |
nx build @tryghost/signup-form |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/announcement-bar |
✅ Succeeded | <1s | View ↗ |
nx build @tryghost/comments-ui |
✅ Succeeded | <1s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin |
✅ Succeeded | 1m 31s | View ↗ |
Additional runs (5) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-16 15:13:38 UTC
no ref - the mobile nav bar linked with a bare relative href (`<a href="posts">`), so tapping an item navigated to a real path and forced a full page load (reloading admin, with a white flash in dark mode) - build a hash href (`#/posts`) like the canonical NavMenuItem.Link does, so navigation stays client-side
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/admin/src/layout/app-sidebar/mobile-nav-bar.tsx`:
- Line 18: The href variable construction uses optional chaining with a template
literal, but this still allows undefined values to be coerced into the string
"undefined", resulting in href="`#/undefined`" being rendered when the to prop is
absent. Add a guard condition to check if to exists before building the href
string, either by using a null coalescing operator (like to ?? ''), by wrapping
the href construction in a conditional check, or by making the to prop
mandatory. Apply this same guard wherever href is used to ensure no "undefined"
paths are generated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 883c781d-1a8d-4b34-964f-1c4864cca4af
📒 Files selected for processing (1)
apps/admin/src/layout/app-sidebar/mobile-nav-bar.tsx
| const isActive = useIsActiveLink({ path: to, activeOnSubpath }); | ||
| // Use a hash route (e.g. "#/posts") so navigation stays client-side; a bare | ||
| // relative href like "posts" triggers a full page load and reloads all of admin. | ||
| const href = `#/${to?.replace(/^\/?#\//, '')}`; |
There was a problem hiding this comment.
Avoid generating #/undefined when to is absent.
At Line 18, optional chaining still allows template-literal coercion to "undefined", so Line 28 can render href="#/undefined" if to is ever omitted. Guard the value (or require to) before building href.
Suggested fix
interface MobileNavBarButtonProps extends Omit<React.ComponentProps<typeof Button>, 'asChild'> {
- to?: string;
+ to: string;
activeOnSubpath?: boolean;
}
@@
- const href = `#/${to?.replace(/^\/?#\//, '')}`;
+ const href = `#/${to.replace(/^\/?#\//, '')}`;Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/admin/src/layout/app-sidebar/mobile-nav-bar.tsx` at line 18, The href
variable construction uses optional chaining with a template literal, but this
still allows undefined values to be coerced into the string "undefined",
resulting in href="`#/undefined`" being rendered when the to prop is absent. Add a
guard condition to check if to exists before building the href string, either by
using a null coalescing operator (like to ?? ''), by wrapping the href
construction in a conditional check, or by making the to prop mandatory. Apply
this same guard wherever href is used to ensure no "undefined" paths are
generated.

Two fixes to the mobile admin nav bar (
mobile-nav-bar.tsx), both stemming from it not following the patterns the rest of the sidebar nav already uses.1. Tapping an item reloaded all of admin
The nav buttons linked with a bare relative href —
<a href="posts">— so the browser navigated to a real path (/ghost/posts) and did a full page load: the whole admin reloaded, with a white-screen flash before the dark theme re-applied.The canonical
NavMenuItem.Linkavoids this by building a hash href (#/posts), which is a client-side hash change. This now does the same:2. Active item rendered a light pill in dark mode
The active/hover state hardcoded
bg-gray-200— a fixed light gray that ignores the theme — so the active item showed as a bright pill on the dark bar (with a low-contrast icon). Every other style here already uses theme tokens (bg-sidebar,border-sidebar-border); the active state just got a literal gray.Swapped to the theme-aware sidebar tokens (
bg-sidebar-accent/text-sidebar-accent-foreground), matching shade's canonicalSidebarMenuButton.Notes
A grep of the admin shell turned up a few other hardcoded grays (
text-gray-700on some sidebar action buttons;text-gray-*withdark:variants in the header). Those are text colors on hover/focus or already carrydark:overrides, so they're milder and more of a design call — left out to keep this focused.