Skip to content

Hid the admin sidebar for the automations/:id React route#28558

Open
cmraible wants to merge 5 commits into
mainfrom
codex/toggle-admin-sidebar-in-react
Open

Hid the admin sidebar for the automations/:id React route#28558
cmraible wants to merge 5 commits into
mainfrom
codex/toggle-admin-sidebar-in-react

Conversation

@cmraible

@cmraible cmraible commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

closes https://linear.app/ghost/issue/NY-1295/wire-up-ability-to-toggle-admin-sidebar-just-in-react-so-its-off-for

Summary

  • Add useAdminSidebarVisibility to combine Ember sidebar state with React route handles
  • Hide the admin sidebar and mobile nav when a matched route opts out
  • Mark the automation editor route to hide the sidebar
  • Add unit coverage for the new visibility logic

Why

The automations editor needs a fullscreen canvas. The previous CSS-only approach visually covered the admin sidebar by positioning the canvas above it, but the sidebar still existed underneath the page. That is not semantically correct: the route behaves like a sidebarless fullscreen editor, while the DOM still contains navigation that is not meant to be available on that screen.

That also creates an accessibility problem. If the sidebar remains mounted and is only visually obscured, assistive technology and keyboard navigation can still encounter sidebar links that are hidden from sighted users. The visual page and the accessibility tree can disagree about what UI is present.

This changes the sidebar behavior into a route-level layout decision. React routes can opt out with handle: {hideAdminSidebar: true}, and the admin layout derives whether to render the sidebar from the active route matches. For the automations editor, the sidebar is no longer just covered by the canvas; it is omitted from the rendered layout for that route.

The hook still respects the existing Ember bridge sidebar state so current Ember-driven fullscreen behavior, such as the post editor, keeps working while React routes gain a dedicated pattern that does not add new Ember coupling.

Testing

  • Added unit tests for default visibility and route-based hiding behavior
  • Ran pnpm --filter @tryghost/admin test:unit -- src/layout/sidebar-visibility.test.tsx
  • Ran pnpm --filter @tryghost/admin typecheck

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cfd4b0fc-4baf-4abf-bf47-3155ef006af1

📥 Commits

Reviewing files that changed from the base of the PR and between a45164c and 0c64c47.

📒 Files selected for processing (3)
  • apps/admin-x-framework/src/index.ts
  • apps/admin/src/layout/sidebar-visibility.ts
  • apps/posts/src/routes.tsx

Walkthrough

This PR introduces a route-aware sidebar visibility mechanism for the admin app. A new useAdminSidebarVisibility hook is added that combines the existing Ember sidebar visibility state with React route metadata, allowing individual routes to opt out of displaying the admin sidebar via a hideAdminSidebar handle flag. The admin layout and mobile nav bar components are updated to use this new hook instead of the bridge hook, and the sidebar component rendering becomes conditional. The automations route is configured to hide the sidebar when active.

Suggested reviewers

  • rob-ghost
  • jonatansberg
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: hiding the admin sidebar for a specific React route (automations/:id), which is the primary objective of the pull request.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation for the sidebar visibility changes, the implementation approach, and testing performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/toggle-admin-sidebar-in-react

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud

nx-cloud Bot commented Jun 12, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit a45164c

Command Status Duration Result
nx run @tryghost/admin-x-settings:test:acceptance ✅ Succeeded 10m 2s View ↗
nx build @tryghost/announcement-bar ✅ Succeeded <1s View ↗
nx build @tryghost/portal ✅ Succeeded <1s View ↗
nx build @tryghost/activitypub ✅ Succeeded 1s View ↗
nx build @tryghost/sodo-search ✅ Succeeded <1s View ↗
nx build @tryghost/signup-form ✅ Succeeded <1s View ↗
nx build @tryghost/admin-toolbar ✅ Succeeded <1s View ↗
nx build @tryghost/comments-ui ✅ Succeeded <1s View ↗
Additional runs (8) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-06-12 23:17:27 UTC

cmraible added 3 commits June 12, 2026 18:43
ref https://linear.app/tryghost/issue/NY-1295/

- keep the React sidebar visibility helper aligned with the TypeScript style guide by using type aliases for object shapes
ref https://linear.app/tryghost/issue/NY-1295/

- avoid setting explicit undefined properties in the sidebar visibility route match test data
ref https://linear.app/tryghost/issue/NY-1295/

- avoid a type assertion when reading React Router handle metadata by narrowing the route handle before checking the sidebar flag
@cmraible cmraible marked this pull request as ready for review June 12, 2026 22:54
@cmraible cmraible changed the title Hide the admin sidebar for selected React routes Hid the admin sidebar for the automations/:id React route Jun 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/posts/src/routes.tsx (1)

81-81: 📐 Maintainability & Code Quality | ⚡ Quick win

Consider adding type safety to the route handle.

The handle object could benefit from a type annotation to provide IDE autocomplete and catch typos at compile time.

✨ Suggested improvement with type annotation

Import the type at the top of the file:

 import {ErrorPage} from '`@tryghost/shade/primitives`';
 import {RouteObject, lazyComponent} from '`@tryghost/admin-x-framework`';
+import type {AdminRouteHandle} from '`@/layout/sidebar-visibility`';

Then apply the type using satisfies:

                 {
                     path: ':id',
-                    handle: {hideAdminSidebar: true},
+                    handle: {hideAdminSidebar: true} satisfies AdminRouteHandle,
                     lazy: lazyComponent(() => import('`@views/Automations/editor`'))
                 }

This provides compile-time safety while preserving the runtime type checking.

🤖 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/posts/src/routes.tsx` at line 81, Add a type annotation for the route
handle to get compile-time safety: import the appropriate Handle type used by
your routing framework at the top of the file, then annotate the route's handle
object (the one containing hideAdminSidebar) using either a type assertion or
the "satisfies" operator so the handle conforms to that Handle type; update the
route definition where handle: { hideAdminSidebar: true } is declared (in
apps/posts/src/routes.tsx) to use that typed handle so IDE autocomplete and typo
checking work.
🤖 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.

Nitpick comments:
In `@apps/posts/src/routes.tsx`:
- Line 81: Add a type annotation for the route handle to get compile-time
safety: import the appropriate Handle type used by your routing framework at the
top of the file, then annotate the route's handle object (the one containing
hideAdminSidebar) using either a type assertion or the "satisfies" operator so
the handle conforms to that Handle type; update the route definition where
handle: { hideAdminSidebar: true } is declared (in apps/posts/src/routes.tsx) to
use that typed handle so IDE autocomplete and typo checking work.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8a5fdf89-b2bb-408b-8f80-95a352cf18f1

📥 Commits

Reviewing files that changed from the base of the PR and between c490404 and a45164c.

📒 Files selected for processing (5)
  • apps/admin/src/layout/admin-layout.tsx
  • apps/admin/src/layout/app-sidebar/mobile-nav-bar.tsx
  • apps/admin/src/layout/sidebar-visibility.test.tsx
  • apps/admin/src/layout/sidebar-visibility.ts
  • apps/posts/src/routes.tsx

ref https://linear.app/tryghost/issue/NY-1295/

- share the admin route handle shape through admin-x-framework so React route metadata gets compile-time checking where it is declared
@cmraible cmraible requested a review from troyciesco June 12, 2026 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant