feat(admin): allow trusted plugins to add editor panels - #2187
Conversation
🦋 Changeset detectedLatest commit: a9614d9 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
added
the
review/needs-rereview
There was a problem hiding this comment.
Re-review of PR #2187.
Approach: This remains the right, well-scoped extension for the problem in #2227. A trusted-plugin React export contract, host-owned sidebar sections, manifest lifecycle filtering, collection/role filtering, deterministic ordering, and render-error isolation all fit EmDash's architecture.
Status of prior findings:
- ✅ The Kumo
Buttonfinding from the last review is resolved —ContentEditorPanelBoundarynow importsButtonfrom@cloudflare/kumo. ⚠️ The process note remains open: the PR checklist item "New features link to an approved Discussion" is still unchecked. The description links to Discussion #2105, but perAGENTS.mda maintainer-approved Discussion is required before merging a feature.
What I checked:
- Panel contract, runtime validation, and resolver behavior in
packages/admin/src/lib/content-editor-panels.tsx. - Integration into
ContentSettingsPaneland themanifestpass-through fromContentEditor. - Dynamic section ordering in
SortableContentSettingsSectionsandcontent-settings-layout.ts. - Lingui wrapping, Kumo component use, RTL-safe Tailwind classes, and comment discipline against
AGENTS.md. - Test coverage for filtering, ordering, deduplication, error containment, and
ContentSettingsPanelintegration. - Changeset quality and the
react-admin.mdxdocumentation update. - No server routes, SQL, content-table queries, or logged-out query counts are affected.
Headline conclusion: The implementation is clean and the new tests/docs are appropriate. One minor code suggestion: the error-boundary Retry button currently only resets the error flag and does not remount the failed subtree, so deterministic render errors do not actually clear on retry. The fix is cheap (add a resetKey and key the children). Once that is addressed and the Discussion checkbox is confirmed/ticked, this is ready to merge.
| size="sm" | ||
| className="mt-1" | ||
| onClick={() => this.setState({ hasError: false })} |
There was a problem hiding this comment.
[suggestion] Clicking Retry only flips hasError back to false. React error boundaries unmount the failed subtree, so rendering the same children again without a changed key will usually recreate the same panel and hit the same deterministic render error, making the button a no-op for most failures.
Add a resetKey to the boundary state and use it to key the children so Retry actually remounts the panel:
| size="sm" | |
| className="mt-1" | |
| onClick={() => this.setState({ hasError: false })} | |
| interface ContentEditorPanelBoundaryState { | |
| hasError: boolean; | |
| resetKey: number; | |
| } | |
| /** Prevents one faulty trusted panel from unmounting the content editor. */ | |
| export class ContentEditorPanelBoundary extends React.Component< | |
| ContentEditorPanelBoundaryProps, | |
| ContentEditorPanelBoundaryState | |
| > { | |
| override state: ContentEditorPanelBoundaryState = { hasError: false, resetKey: 0 }; | |
| static getDerivedStateFromError(): ContentEditorPanelBoundaryState { | |
| return { hasError: true, resetKey: 0 }; | |
| } | |
| override componentDidCatch(error: Error, info: React.ErrorInfo): void { | |
| console.error( | |
| `Plugin "${this.props.pluginId}" failed while rendering content editor panel "${this.props.panelId}".`, | |
| error, | |
| info, | |
| ); | |
| } | |
| override render(): React.ReactNode { | |
| if (!this.state.hasError) { | |
| return ( | |
| <React.Fragment key={this.state.resetKey}> | |
| {this.props.children} | |
| </React.Fragment> | |
| ); | |
| } | |
| return ( | |
| <div role="alert" className="text-sm text-kumo-subtle"> | |
| <p> | |
| <Trans>Plugin panel unavailable.</Trans> | |
| </p> | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| size="sm" | |
| className="mt-1" | |
| onClick={() => | |
| this.setState((prev) => ({ | |
| hasError: false, | |
| resetKey: prev.resetKey + 1, | |
| })) | |
| } | |
| > | |
| <Trans>Retry</Trans> | |
| </Button> | |
| </div> | |
| ); | |
| } | |
| } |
70ba489 to
b160e7e
Compare
|
Added It covers the gap behind the resetKey suggestion. The existing Retry test only had a panel failing on its first render, so it never mounted. The new one mounts, renders, then throws: on Retry the panel returns as a second instance with its own state reset, because React discards the subtree when the boundary catches. There is no retained instance left for a key to replace. Both Retry tests fail if the button is made a no-op. |
77f1ca6 to
b160e7e
Compare
b160e7e to
a9614d9
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: #2105
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