Added frontend admin toolbar#28058
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new frontend admin-toolbar package (Preact) with build config, UMD bundle, components, styles, action/link helpers, auth iframe + admin API, storage helpers, and a bootstrap entrypoint that renders into shadow DOM. Integrates server-side: middleware to set/validate a signed toolbar cookie, head-scripts service to inject the toolbar script with data attributes, a public route to serve the bundle, build-step copying of the prebuilt UMD into the core public assets, and unit/integration tests for client and server behavior. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/admin/src/layout/app-sidebar/user-menu.tsx (1)
16-28: ⚡ Quick winConsider extracting the helper to a shared module.
Both
nav-main.tsxanduser-menu.tsxdefine similargetAdminToolbarUrlhelpers with slightly different signatures:
nav-main.tsx:(url?: string) => string | undefineduser-menu.tsx:(url: string) => stringThis duplication could lead to maintenance drift. Consider extracting a single shared helper (e.g., to a
utilsorhelpersmodule) to maintain consistency.Also applies to: 205-205
🤖 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/user-menu.tsx` around lines 16 - 28, Extract the duplicated getAdminToolbarUrl logic into a single exported helper in a shared module (e.g., helpers or utils) and update both implementations to import and use that shared function; unify the signature to accept url?: string and return string | undefined (or choose the consistent nullable/empty behavior your app expects), implement the same URL manipulation (new URL(url) with searchParams.set("admin","1") and safe try/catch fallback), export it (e.g., export function getAdminToolbarUrl(url?: string): string | undefined) and replace the local functions in nav-main.tsx and user-menu.tsx with imports from the new module, updating call sites to handle the unified return type.
🤖 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-toolbar/package.json`:
- Around line 20-24: The package.json scripts call binaries directly; update the
"scripts" entries to invoke binaries through pnpm: change "build" to use "pnpm
exec vite build", change "lint" to "pnpm exec eslint src --ext .js --cache", and
change "test" to call the local build via "pnpm run build" then run mocha via
pnpm like "pnpm exec mocha --exit --trace-warnings --timeout=60000
\"test/**/*.test.js\""; modify the "build", "lint", and "test" script values
accordingly.
---
Nitpick comments:
In `@apps/admin/src/layout/app-sidebar/user-menu.tsx`:
- Around line 16-28: Extract the duplicated getAdminToolbarUrl logic into a
single exported helper in a shared module (e.g., helpers or utils) and update
both implementations to import and use that shared function; unify the signature
to accept url?: string and return string | undefined (or choose the consistent
nullable/empty behavior your app expects), implement the same URL manipulation
(new URL(url) with searchParams.set("admin","1") and safe try/catch fallback),
export it (e.g., export function getAdminToolbarUrl(url?: string): string |
undefined) and replace the local functions in nav-main.tsx and user-menu.tsx
with imports from the new module, updating call sites to handle the unified
return type.
🪄 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: 85c27896-68ed-4e8f-9041-d127cb454978
⛔ Files ignored due to path filters (3)
apps/admin-toolbar/umd/admin-toolbar.min.jsis excluded by!**/*.min.jsghost/core/core/frontend/public/admin-toolbar.min.jsis excluded by!**/*.min.jspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
apps/admin-toolbar/LICENSEapps/admin-toolbar/README.mdapps/admin-toolbar/package.jsonapps/admin-toolbar/src/actions.jsapps/admin-toolbar/src/auth.jsapps/admin-toolbar/src/body-offset.jsapps/admin-toolbar/src/components.jsapps/admin-toolbar/src/config.jsapps/admin-toolbar/src/constants.jsapps/admin-toolbar/src/icons.jsapps/admin-toolbar/src/index.jsapps/admin-toolbar/src/links.jsapps/admin-toolbar/src/storage.jsapps/admin-toolbar/src/styles.jsapps/admin-toolbar/src/user.jsapps/admin-toolbar/test/admin-toolbar.test.jsapps/admin-toolbar/vite.config.mjsapps/admin/src/layout/app-sidebar/nav-main.tsxapps/admin/src/layout/app-sidebar/user-menu.tsxghost/admin/app/components/gh-site-iframe.jsghost/core/bin/minify-assets.jsghost/core/core/frontend/helpers/ghost_head.jsghost/core/core/frontend/services/staff-frontend-tools/head-scripts.jsghost/core/core/frontend/web/middleware/admin-toolbar.jsghost/core/core/frontend/web/middleware/frontend-caching.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/routers/serve-public-file.jsghost/core/core/frontend/web/site.jsghost/core/package.jsonghost/core/test/unit/frontend/helpers/ghost-head.test.jsghost/core/test/unit/frontend/web/middleware/admin-toolbar.test.jsghost/core/test/unit/frontend/web/middleware/frontend-caching.test.jspnpm-workspace.yaml
JohnONolan
force-pushed
the
codex/frontend-admin-toolbar
branch
2 times, most recently
from
6b49587 to
137cb00
Compare
Contributor
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 `@ghost/core/package.json`:
- Line 49: Update the package.json scripts to invoke executables through pnpm
instead of calling node or vitest directly: change the "build:assets:js" script
so it uses pnpm to run the minifier (replace the direct "node
bin/minify-assets.js" invocation with a pnpm-based invocation), and likewise
update the scripts that call vitest (referenced in the same file around the
other script entries) to use "pnpm vitest" instead of calling vitest directly;
ensure you modify the script strings that reference build:assets:js and the
vitest-related script names so all executables are run via pnpm per repo policy.
🪄 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: 0c6fe516-52a1-4905-8e87-00ed841738f0
⛔ Files ignored due to path filters (2)
apps/admin-toolbar/umd/admin-toolbar.min.jsis excluded by!**/*.min.jsghost/core/core/frontend/public/admin-toolbar.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (32)
apps/admin-toolbar/LICENSEapps/admin-toolbar/README.mdapps/admin-toolbar/package.jsonapps/admin-toolbar/src/actions.jsapps/admin-toolbar/src/auth.jsapps/admin-toolbar/src/body-offset.jsapps/admin-toolbar/src/components.jsapps/admin-toolbar/src/config.jsapps/admin-toolbar/src/constants.jsapps/admin-toolbar/src/icons.jsapps/admin-toolbar/src/index.jsapps/admin-toolbar/src/links.jsapps/admin-toolbar/src/storage.jsapps/admin-toolbar/src/styles.jsapps/admin-toolbar/src/user.jsapps/admin-toolbar/test/admin-toolbar.test.jsapps/admin-toolbar/vite.config.mjsapps/admin/src/layout/app-sidebar/nav-main.tsxapps/admin/src/layout/app-sidebar/user-menu.tsxghost/admin/app/components/gh-site-iframe.jsghost/core/bin/minify-assets.jsghost/core/core/frontend/helpers/ghost_head.jsghost/core/core/frontend/services/staff-frontend-tools/head-scripts.jsghost/core/core/frontend/web/middleware/admin-toolbar.jsghost/core/core/frontend/web/middleware/frontend-caching.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/routers/serve-public-file.jsghost/core/core/frontend/web/site.jsghost/core/package.jsonghost/core/test/unit/frontend/helpers/ghost-head.test.jsghost/core/test/unit/frontend/web/middleware/admin-toolbar.test.jsghost/core/test/unit/frontend/web/middleware/frontend-caching.test.js
✅ Files skipped from review due to trivial changes (1)
- apps/admin-toolbar/LICENSE
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/admin/src/layout/app-sidebar/user-menu.tsx (1)
16-28: ⚡ Quick winExtract
getAdminToolbarUrlinto a shared sidebar utility.This helper is duplicated in
nav-main.tsxanduser-menu.tsx; centralizing it avoids divergence in toolbar URL behavior.🤖 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/user-menu.tsx` around lines 16 - 28, Extract the duplicated getAdminToolbarUrl function (currently in user-menu.tsx and nav-main.tsx) into a single shared utility module (e.g., export function getAdminToolbarUrl(...) from a new sidebar utility file), update both user-menu.tsx and nav-main.tsx to import and use that shared function, and remove the local copies; ensure the new utility preserves the exact try/catch behavior (returns "" for falsy input, attempts new URL(), sets search param "admin" to "1", returns href, and falls back to returning the original url on parse error) and run any relevant linter/tests to confirm no regressions.
🤖 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-toolbar/src/index.js`:
- Around line 51-53: The await on the iframe load currently can hang forever
because it waits on a Promise created with frame.addEventListener('load',
resolve, {once: true});; modify this to race the load event against a timeout:
create a Promise that either resolves on frame.addEventListener('load', ...) or
rejects/ resolves with an error after a configurable timeout (e.g., 5–10s),
ensure you clean up the event listener and clear the timeout in both paths, and
surface the timeout error so the caller can run fallback/cleanup logic
(referencing the existing frame and the load event listener).
In `@pnpm-workspace.yaml`:
- Line 60: The catalog package currently lists preact as a caret-range
dependency ("preact: ^10.29.2") which violates catalogMode: strict; update the
catalog dependency entry for preact to an exact version (remove the caret) so it
reads the exact release (10.29.2) instead of ^10.29.2; locate the preact entry
under the catalog package in pnpm-workspace.yaml (symbol: catalog.preact) and
replace the version specifier with the exact version.
---
Nitpick comments:
In `@apps/admin/src/layout/app-sidebar/user-menu.tsx`:
- Around line 16-28: Extract the duplicated getAdminToolbarUrl function
(currently in user-menu.tsx and nav-main.tsx) into a single shared utility
module (e.g., export function getAdminToolbarUrl(...) from a new sidebar utility
file), update both user-menu.tsx and nav-main.tsx to import and use that shared
function, and remove the local copies; ensure the new utility preserves the
exact try/catch behavior (returns "" for falsy input, attempts new URL(), sets
search param "admin" to "1", returns href, and falls back to returning the
original url on parse error) and run any relevant linter/tests to confirm no
regressions.
🪄 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: 1b5e7c74-4c15-410b-93b1-680614504706
⛔ Files ignored due to path filters (3)
apps/admin-toolbar/umd/admin-toolbar.min.jsis excluded by!**/*.min.jsghost/core/core/frontend/public/admin-toolbar.min.jsis excluded by!**/*.min.jspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
apps/admin-toolbar/LICENSEapps/admin-toolbar/README.mdapps/admin-toolbar/package.jsonapps/admin-toolbar/src/actions.jsapps/admin-toolbar/src/auth.jsapps/admin-toolbar/src/body-offset.jsapps/admin-toolbar/src/components.jsapps/admin-toolbar/src/config.jsapps/admin-toolbar/src/constants.jsapps/admin-toolbar/src/icons.jsapps/admin-toolbar/src/index.jsapps/admin-toolbar/src/links.jsapps/admin-toolbar/src/storage.jsapps/admin-toolbar/src/styles.jsapps/admin-toolbar/src/user.jsapps/admin-toolbar/test/admin-toolbar.test.jsapps/admin-toolbar/vite.config.mjsapps/admin/src/layout/app-sidebar/nav-main.tsxapps/admin/src/layout/app-sidebar/user-menu.tsxghost/admin/app/components/gh-site-iframe.jsghost/core/bin/minify-assets.jsghost/core/core/frontend/helpers/ghost_head.jsghost/core/core/frontend/services/staff-frontend-tools/head-scripts.jsghost/core/core/frontend/web/middleware/admin-toolbar.jsghost/core/core/frontend/web/middleware/frontend-caching.jsghost/core/core/frontend/web/middleware/index.jsghost/core/core/frontend/web/routers/serve-public-file.jsghost/core/core/frontend/web/site.jsghost/core/package.jsonghost/core/test/unit/frontend/helpers/ghost-head.test.jsghost/core/test/unit/frontend/web/middleware/admin-toolbar.test.jsghost/core/test/unit/frontend/web/middleware/frontend-caching.test.jspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- apps/admin-toolbar/LICENSE
- apps/admin-toolbar/README.md
JohnONolan
force-pushed
the
codex/frontend-admin-toolbar
branch
from
9307271 to
bc3c09d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
JohnONolan
force-pushed
the
codex/frontend-admin-toolbar
branch
2 times, most recently
from
b660fb5 to
cc21788
Compare
Member
Suggestions
|
ErisDS
force-pushed
the
codex/frontend-admin-toolbar
branch
2 times, most recently
from
7d8c89e to
f47d9b4
Compare
Adds a staff-only frontend toolbar that can be enabled from Admin view-site links while keeping public HTML cacheable for normal visitors. The toolbar is built as its own frontend package so Ghost core consumes a built UMD artifact rather than package internals, and staff frontend script injection is isolated behind a small core boundary.
Updates the new admin toolbar package scripts to invoke workspace binaries through pnpm, matching repository conventions.
Regenerates the new admin toolbar package lockfile entry against current main so frozen CI installs resolve the Vite peer set correctly.
Uses pnpm to invoke the core asset minifier so the new admin toolbar build path follows workspace script conventions.
Use getFrontendAppConfig with a jsDelivr URL like all other public apps (portal, comments-ui, sodo-search) instead of a custom copy-into-core pipeline. Removes the local serve route, minify-assets copy step, and committed build artifact from ghost/core. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The theme customisation, announcement bar, portal, and offer preview panels all fetch site content that could include the admin toolbar script. Added admin_toolbar=0 to suppress the toolbar in all previews. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers resource type detection, context-dependent attributes, and the implicit comments-enabled default. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The middleware already suppresses the toolbar server-side when ?admin_toolbar=0 is in the URL. This adds the same check client-side so the toolbar does not render even if the script was injected from cache. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The admin_toolbar=0 suppression param added to preview URLs caused Playwright route matching to fail because page.route used an exact URL match. Changed to a glob pattern so extra query params are allowed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ErisDS
force-pushed
the
codex/frontend-admin-toolbar
branch
from
f47d9b4 to
54e15dc
Compare
Deduplicated the getAdminToolbarUrl function from nav-main.tsx and user-menu.tsx into a shared utility module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removed the separate staff-frontend-tools service directory and inlined the admin toolbar script assembly into ghost_head.js, matching the pattern used by portal, search, and announcement-bar. Also added the missing crossorigin="anonymous" attribute for CDN consistency. Co-Authored-By: Claude Opus 4.6 <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
2 participants
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.
Summary
@tryghost/admin-toolbarpackage that owns the toolbar source, tests, Vite build, and UMD artifact.ghost_headinjects staff-only scripts only after the frontend marker is present.toolbar.mp4
Architecture
The toolbar is not rendered into public HTML by default. Admin links to the frontend add
?admin=1, which is handled by the new frontend middleware. That middleware creates a short-lived, signedghost-admin-toolbarmarker cookie on the frontend domain and redirects back to the clean URL. The marker is intentionally not a staff session and contains no personalized data; it only says that this browser should be offered staff frontend tooling.On subsequent frontend requests, the middleware validates the marker and sets
res.locals.staffFrontendToolsEnabled.ghost_headdelegates staff-only script selection tostaff-frontend-tools/head-scripts, which emits the non-personaladmin-toolbar.min.jsscript and page metadata needed to build the correct toolbar actions.The browser bundle still verifies the real staff session through the existing hidden
/ghost/auth-frame/bridge before rendering. It callsgetUser, requires an Owner, Administrator, or Editor role, then mounts the toolbar in a Shadow DOM so theme CSS cannot affect it. This keeps normal visitor pages cacheable and prevents staff-specific data from being embedded in server-rendered HTML.The Admin "view site" iframe also passes
admin=1&admin_toolbar=0. That seeds the frontend marker cookie, but the middleware suppresses staff tool injection inside iframe responses using the explicit query parameter andSec-Fetch-Dest: iframe. This lets Admin enable the toolbar for normal frontend visits without rendering the toolbar inside the Admin preview iframe.Package boundary
The toolbar source now lives in
apps/admin-toolbar. The package owns its source modules, Vite build, tests, README/LICENSE, andumd/admin-toolbar.min.js. Ghost core no longer builds fromapps/admin-toolbar/src;ghost/corebuilds the package first and then copies the package UMD artifact intocore/frontend/public/admin-toolbar.min.js. This keeps core responsible for activation, caching, serving, and head injection, while the package owns the toolbar runtime.