[Fix/197 plugin settings link] fix(admin): correct settings link to plugin root#227
[Fix/197 plugin settings link] fix(admin): correct settings link to plugin root#227WebDevByCam wants to merge 3 commits intoemdash-cms:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 299a504 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Fixes the Admin UI “Settings” link for plugins so it no longer navigates to a hard-coded /settings subpath (addressing #197), aiming to support plugins whose primary admin page is the root or another path.
Changes:
- Update the plugin “Settings” button navigation target in the plugin manager UI.
- Add a changeset for a patch release of
@emdash-cms/admin.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/admin/src/components/PluginManager.tsx | Changes the Settings link destination for plugin admin pages. |
| .changeset/fix-plugin-settings-link.md | Adds a patch changeset describing the navigation fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {plugin.hasAdminPages && plugin.enabled && ( | ||
| <Link to="/plugins/$pluginId/$" params={{ pluginId: plugin.id, _splat: "settings" }}> | ||
| <Link to="/plugins/$pluginId/$" params={{ pluginId: plugin.id, _splat: "" }}> | ||
| <Button variant="ghost" shape="square" aria-label="Settings"> | ||
| <Gear className="h-4 w-4" /> | ||
| <span className="sr-only">Settings</span> |
There was a problem hiding this comment.
Setting _splat to an empty string forces navigation to the plugin pagePath "/". Many plugins only expose admin pages at non-root paths (e.g. "/settings", "/status"), so this change will make the Settings button land on an empty/404 plugin page for those plugins (regression from the previous behavior).
Instead of hard-coding "settings" or "", derive the destination from the plugin’s declared adminPages in the manifest (e.g. first page), and convert that path into the router’s _splat form (strip the leading "/"; use empty/undefined for "/"). Also consider updating the aria-label/sr-only text to match the actual destination (page label or a generic "Open plugin").
| {plugin.hasAdminPages && plugin.enabled && ( | ||
| <Link to="/plugins/$pluginId/$" params={{ pluginId: plugin.id, _splat: "settings" }}> | ||
| <Link to="/plugins/$pluginId/$" params={{ pluginId: plugin.id, _splat: "" }}> | ||
| <Button variant="ghost" shape="square" aria-label="Settings"> |
There was a problem hiding this comment.
There are existing PluginManager tests, but they only assert the Settings button exists and the mocked Link ignores params, so this navigation change isn’t covered. Please add/update tests to verify the Settings link targets the correct plugin admin page path (at least cases for adminPages root "/" vs "/settings" vs a nested path).
What does this PR do?
Fixes the plugin "Settings" link in the Admin UI so it navigates to the plugin's root page instead of a hard-coded "settings" path. This makes plugin settings navigation work for plugins that expose root pages or nested pages, and avoids broken routes when the plugin's page structure differs from the previous assumption.
Closes #197
Type of change
Checklist
pnpm typecheckpassespnpm --silent lint:json | jq '.diagnostics | length'returns 0pnpm testpasses (or targeted tests for my change)pnpm formathas been run