feat(admin): allow trusted plugins to add content list columns - #2195
feat(admin): allow trusted plugins to add content list columns#2195logelog wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 5760ef3 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 |
Scope checkThis PR changes 586 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: |
There was a problem hiding this comment.
This is a focused, well-scoped addition to the trusted-plugin admin surface. It solves the right problem (display-only content-list columns contributed by trusted React plugins) in the right way: EmDash keeps ownership of the table while plugins provide header/cell components;filtering, sorting, and search are deliberately left out because they need server-backed semantics. The change follows the same trusted-plugin model already used for pages, widgets, and field widgets, and it is backed by an approved Discussion (#2048).
I checked the new resolver, the boundary component, the ContentList integration, the router wiring, the test coverage, and the changeset. The logic is sound: disabled/stale plugins are omitted, role and collection filters work, malformed/duplicate definitions are isolated, and error boundaries keep a broken column from crashing the list. No logic bugs, security issues, regressions, or logged-out query-count impacts were found.
There are three AGENTS.md import-extension violations on changed/new lines: two new internal imports omit the required .js extension. Fixing those is the only blocker.
| import { Trans } from "@lingui/react/macro"; | ||
| import * as React from "react"; | ||
|
|
||
| import type { AdminManifest, ContentItem } from "./api"; |
There was a problem hiding this comment.
[needs fixing] Internal ESM imports must use a .js extension per AGENTS.md. This new file imports from ./api without it.
| import type { AdminManifest, ContentItem } from "./api"; | |
| import type { AdminManifest, ContentItem } from "./api.js"; |
| ContentDateField, | ||
| ContentItem, | ||
| TrashedContentItem, | ||
| } from "../lib/api"; |
There was a problem hiding this comment.
[needs fixing] This changed import from ../lib/api is missing the required .js extension for internal ESM imports.
| } from "../lib/api"; | |
| } from "../lib/api.js"; |
| ContentListColumnHeaderContext, | ||
| ContentListColumnCellContext, | ||
| ContentListColumnExtension, | ||
| } from "./lib/content-list-columns"; |
There was a problem hiding this comment.
[needs fixing] The new re-export from ./lib/content-list-columns is an internal ESM import and needs the .js extension.
| } from "./lib/content-list-columns"; | |
| } from "./lib/content-list-columns.js"; |
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. |
ee1f044 to
0a83f6d
Compare
0a83f6d to
8825b20
Compare
8825b20 to
ffe1365
Compare
ffe1365 to
139939b
Compare
36d5c8c to
bacadcf
Compare
There was a problem hiding this comment.
Re-review of the trusted-plugin content-list column extension. The approach is sound: EmDash keeps full ownership of the table, pagination, row actions, loading/empty states, and sort/filter contracts while letting trusted React plugins and collection-configured scalar fields contribute display-only columns. This fits EmDash’s existing manifest-driven plugin architecture without leaking host responsibilities to plugins.
The only prior finding—the extensionless barrel-file export in packages/admin/src/components/index.ts—is now fixed (./ContentList.js).
I re-read the full diff end-to-end:
ContentListcorrectly integrates collection-configured columns between title/status and plugin-contributed columns between date/actions, updatescolSpaneverywhere, and leaves sort/filter/search untouched.resolveContentListColumnsdeterministically orders contributions, filters disabled/stale plugins and unauthorized roles, validates malformed definitions, deduplicates per-plugin IDs, and isolates throwing collection predicates.ContentListColumnBoundarycontains header and cell render failures so one broken plugin cannot unmount the list, and it only renders in the active-content table (not Trash).- User-facing strings are Lingui-wrapped; alignment uses RTL-safe logical Tailwind (
text-start/text-end). - Core manifest build, schema registry persistence, migrations, seed/validate/export, and API schemas all correctly cap list columns at four, restrict them to supported field types, and preserve them round-trip.
- New tests cover configured and plugin columns, formatting, colSpan alignment, error isolation, Trash omission, and the resolver’s filtering/ordering/validation behavior.
I found no logic bugs, security issues, locale-filter misses, broken API-envelope contracts, AGENTS.md convention violations, or logged-out query-count regressions on this pass.
One non-code item remains: per the PR description, the linked Discussion is still awaiting maintainer approval; that is a governance gate outside the code review itself. LGTM.
bacadcf to
760dcb3
Compare
760dcb3 to
9a3ba0b
Compare
|
@logelog If you resolve the conflicts we can get this in |
9a3ba0b to
7652b23
Compare
Thanks a lot! I’ve rebased the branch onto the current main and resolved the conflicts. All checks are green now. |
7652b23 to
86099f1
Compare
ascorbic
left a comment
There was a problem hiding this comment.
Plugin column labels must remain localizable, and the public cell contract must document batching to prevent per-row requests.
| return ( | ||
| <th | ||
| scope="col" | ||
| aria-label={extension.label} |
There was a problem hiding this comment.
This renders the plugin label directly, so an Arabic admin still gets an English column name (and the same untranslated accessible name), even when the plugin catalog contains a translation. Resolve the label through the active Lingui instance, as plugin page labels are, and use that value for the aria label and both fallbacks.
|
|
||
| ## Content-list columns | ||
|
|
||
| Trusted React plugins can add read-only columns to active content collection lists. EmDash keeps ownership of the table, pagination, row actions, and loading and empty states; the plugin supplies only the header metadata and cell content. |
There was a problem hiding this comment.
The cell contract needs the batching constraint from the approved design. A cell that fetches metadata by item id will mount once per row and turn a 20-row page into 20 requests. Document and demonstrate one shared query key or batched request keyed by visibleItems.
86099f1 to
acc1ed3
Compare
d1df443 to
5760ef3
Compare
What does this PR do?
Adds a focused, display-only content-list column extension for trusted React plugins.
Plugins can export typed
contentListColumnsdefinitions for computed metadata such as review statuses, moderation outcomes, or publication readiness. EmDash retains ownership of the table, pagination, row actions, loading states, and empty states while passing each cell the current content item, collection, and locale.The resolver:
Sorting, filtering, and search remain outside this display-only contract because they require server-backed contracts. Search is covered by #2191, indexed sorting by #2212, and indexed filtering by #2213.
#2194 has merged and supplies the shared content-list layout. This branch is now based on
mainand contains only the trusted-plugin extension commits.Approved Discussion: #2048
Wider content-list context: #2397
Closes: N/A
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
Local validation after rebasing:
git diff --checkNo screenshot is included because the extension renders only when a trusted plugin contributes a column.