feat(admin): allow trusted plugins to add editor panels - #2187
Conversation
🦋 Changeset detectedLatest commit: ceb2e5d The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
github-actions
Bot
added
review/needs-review
Scope checkThis PR changes 548 lines across 10 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change is a focused, well-scoped feature that fits EmDash's trusted-plugin model: it adds a contentEditorPanels export contract, resolves/filters panels using PluginAdminContext and the admin manifest, renders them in host-owned settings-sidebar sections, and isolates failures with a React error boundary. The implementation is clean, focused tests and docs are present, and the changeset is appropriate.
One process note: the PR checklist item "New features link to an approved Discussion" is left unchecked even though the description links to Discussion #2105. That box should be ticked, or the approval status confirmed, before merge per the feature PR policy.
One code finding: the new error boundary's retry button is a raw <button> despite the AGENTS.md rule to use Kumo components. It mirrors the existing PluginFieldErrorBoundary pattern, but new code should migrate to Kumo Button instead. Otherwise the changed files are in good shape and the new conventions (Lingui wrapping, RTL-safe logical classes, manifest lifecycle filtering) are followed.
| <button | ||
| type="button" | ||
| className="mt-1 text-xs font-medium text-kumo-brand underline" | ||
| onClick={() => this.setState({ hasError: false })} | ||
| > | ||
| <Trans>Retry</Trans> | ||
| </button> |
There was a problem hiding this comment.
[suggestion] The fallback UI uses a raw <button> with hand-rolled Tailwind classes (text-kumo-brand underline). AGENTS.md requires using Kumo components rather than custom buttons so the control gets Kumo focus rings, sizing, and dark-mode/RTL behavior for free. Import Button from @cloudflare/kumo and use it here (and, separately, migrate the identical pattern in the pre-existing PluginFieldErrorBoundary).
| <button | |
| type="button" | |
| className="mt-1 text-xs font-medium text-kumo-brand underline" | |
| onClick={() => this.setState({ hasError: false })} | |
| > | |
| <Trans>Retry</Trans> | |
| </button> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="sm" | |
| className="mt-1" | |
| onClick={() => this.setState({ hasError: false })} | |
| > | |
| <Trans>Retry</Trans> | |
| </Button> |
github-actions
Bot
added
review/needs-rereview
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
There was a problem hiding this comment.
The feature is a focused, well-scoped extension that fits EmDash's trusted-plugin model: it adds a contentEditorPanels export contract, resolves/filters panels via PluginAdminContext and the admin manifest, renders them in host-owned settings-sidebar sections, and isolates failures with a React error boundary. The approach is right for the problem.
Status of prior findings:
- ✅ The previous Kumo
Buttonfinding is resolved —ContentEditorPanelBoundarynow imports and usesButtonfrom@cloudflare/kumoinstead of a raw<button>. ⚠️ The process note from the last review remains: the PR checklist item "New features link to an approved Discussion" is still unchecked. The description links to Discussion #2105, but per the feature PR policy the box should be ticked (or the approval status explicitly confirmed) before merge.
What I checked:
- Panel contract, validation, manifest lifecycle filtering, collection/role filtering, deterministic ordering, and render-error recovery in
packages/admin/src/lib/content-editor-panels.tsx. - Integration into
ContentSettingsPaneland themanifestpass-through fromContentEditor. - RTL/Tailwind classes, Lingui wrapping, Kumo component use, and comment discipline against
AGENTS.md. - Test coverage in
ContentSettingsPanel.test.tsxand the newcontent-editor-panels.test.tsx. - Changeset quality and documentation in
react-admin.mdx. - No changes touch server routes, SQL, content-table queries, or logged-out query counts.
Headline conclusion: The implementation is clean, the new tests and docs are appropriate, and the changeset is user-facing. No code defects were found. Once the Discussion checkbox is confirmed/ticked, this is ready to merge.
github-actions
Bot
added
review/awaiting-author
c090a03 to
28139f7
Compare
github-actions
Bot
added
review/needs-rereview
github-actions
Bot
removed
the
review/needs-rereview
28139f7 to
18cad00
Compare
18cad00 to
2ccfeaa
Compare
github-actions
Bot
removed
the
review/needs-rereview
60deb9c to
5a3a1ec
Compare
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
Re-review summary
This PR continues to implement the right extension point for #2227: a trusted-plugin React export contract, EmDash-owned sidebar sections/ordering, manifest lifecycle filtering, deterministic tie-breaking, and per-panel error boundaries all fit the admin architecture. The code I re-read is clean and I found no new blocking defects.
Status of prior findings:
- ✅ The Kumo
Buttonimport inContentEditorPanelBoundaryis correct. - ✅ The Retry/remount concern is resolved; the integration test demonstrates that a transient panel failure recovers via a boundary remount on Retry, and a deterministic render error would re-trip regardless of any internal key.
⚠️ The feature still lacks an approved Discussion. The PR body still lists Discussion #2105 as “awaiting maintainer review” and the corresponding checklist item is unchecked. Per the project process for features, maintainer approval of that Discussion must be confirmed before merge. I cannot verify GitHub state from the repo, so please confirm the Discussion is approved before merging.
What I checked:
- The
contentEditorPanelsresolver (packages/admin/src/lib/content-editor-panels.tsx) — validation, collection/role filtering, manifest enablement checks, deterministic ordering, and swallowed-predicate error handling all look correct. - Integration into
ContentSettingsPanel/ContentEditor—manifestpass-through, per-panel error boundary keyed on content identity, and rendering only for saved entries. - Sortable section support for dynamic plugin sections via
content-settings-layout.tsandSortableContentSettingsSections. - Lingui wrapping for host-rendered strings (
Plugin panel unavailable.,Retry), RTL-safeend-/inlineSizeclasses, and comment discipline on changed hunks. - New resolver + integration tests covering filtering, ordering, lifecycle, disabled plugins, error isolation, and remount behavior.
- Changeset and docs update.
- No server routes, SQL, content-table queries, or logged-out hot-path queries are affected.
I did not run pnpm test/lint/typecheck — I reviewed statically as instructed.
github-actions
Bot
added
review/awaiting-author
5a3a1ec to
dcc7c14
Compare
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
Re-review summary
This PR continues the right extension point for #2227. The design—trusted React plugin exports, EmDash-owned sidebar sections/ordering, manifest lifecycle filtering, deterministic tie-breaking, and per-panel error boundaries—fits the admin architecture. The resolver tests and the new ContentSettingsPanel integration tests cover filtering, ordering, disabled plugins, error isolation, and content-identity remounts. No new server routes, SQL, content-table queries, or logged-out hot-path queries are involved.
Status of prior findings:
- ✅ The Kumo
Buttonimport inContentEditorPanelBoundaryis correct. - ✅ Retry/remount behavior is resolved; the integration test demonstrates that a transient failure recovers via boundary remount on Retry, and a deterministic error re-trips cleanly.
⚠️ The approved Discussion requirement is still unverified. The PR body still lists Discussion #2105 as “awaiting maintainer review” and the corresponding checklist item is unchecked. Per the project process for features, maintainer approval of that Discussion must be confirmed before merge. I cannot verify GitHub state from the repo, so please confirm it before merging.
New issue this pass:
- The host renders the plugin-provided panel
titleas a raw string (extension.title). Every user-facing admin string must go through Lingui so a plugin catalog can localize it. The existingSidebar.tsxplugin page labels already use the samei18n._(id)pattern.
I did not run pnpm test/lint/typecheck—this review is static as instructed.
| @@ -385,6 +393,19 @@ export const ContentSettingsPanel = React.memo(function ContentSettingsPanel({ | |||
| }: ContentSettingsPanelProps) { | |||
| const { t } = useLingui(); | |||
There was a problem hiding this comment.
[needs fixing] The panel section heading and drag-handle label are plugin-provided, user-facing strings, but this component only destructures t from useLingui(). To follow the admin localization convention (see Sidebar.tsx plugin page labels), also expose i18n so extension.title can be passed through i18n._() before rendering.
| const { t } = useLingui(); | |
| const { t, i18n } = useLingui(); |
| label={extension.title} | ||
| > | ||
| <div className="min-w-0 p-4"> | ||
| <Text bold as="h3" DANGEROUS_className="mb-4"> | ||
| {extension.title} |
There was a problem hiding this comment.
[needs fixing] The plugin-supplied title is rendered raw in both the sortable section label and the section heading. Because the host owns these strings, they should be run through Lingui so a plugin that loads its catalog into the shared i18n instance gets translated headings (mirroring how plugin page labels are handled).
| label={extension.title} | |
| > | |
| <div className="min-w-0 p-4"> | |
| <Text bold as="h3" DANGEROUS_className="mb-4"> | |
| {extension.title} | |
| const title = i18n._({ id: extension.title, message: extension.title }); | |
| return ( | |
| <SortableContentSettingsSection | |
| key={sectionId} | |
| id={sectionId} | |
| label={title} | |
| > | |
| <div className="min-w-0 p-4"> | |
| <Text bold as="h3" DANGEROUS_className="mb-4"> | |
| {title} | |
| </Text> |
github-actions
Bot
added
review/awaiting-author
dcc7c14 to
d8cbce8
Compare
github-actions
Bot
added
review/needs-rereview
e4b17ce to
07d0434
Compare
The boundary kept its error state across entries, so a panel that threw on one entry stayed replaced by the fallback after navigating to another. Keying it on collection and entry id rebuilds it with the content it renders. Retry already recovers: React unmounts the subtree when the boundary catches, so clearing the flag mounts the panel fresh. The added test asserts that mount count rather than assuming it.
07d0434 to
ceb2e5d
Compare
2 participants
What does this PR do?
Adds a focused
contentEditorPanelsextension contract for trusted React plugins. Plugins can contribute host-framed panels to the saved-entry settings sidebar while EmDash retains layout ownership, collection and role filtering, disabled-plugin lifecycle handling, deterministic ordering, and render-error isolation.New entries keep the native editor unchanged. A failing contribution cannot take down the editor.
Discussion: #2421
Addresses #2227. That issue asks for a plugin-rendered editor panel that can read the rest of the document rather than a single field's value. A contributed panel receives the whole entry, so it can read the title, slug, body, and every other field. The panel is passed the saved entry, so it refreshes on save/autosave rather than on every keystroke.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs - a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
pnpm typecheckpnpm lint- 0 warnings, 0 errorspnpm format