feat(admin): explicit collection order in the sidebar - #2267
Conversation
🦋 Changeset detectedLatest commit: ae7c280 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
area/admin
review/needs-review
Scope checkThis PR changes 692 lines across 22 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. |
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. |
@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: |
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| "@emdash-cms/core": minor | |||
There was a problem hiding this comment.
| "@emdash-cms/core": minor | |
| "emdash": minor |
|
Thanks! Can you fix the changeset, rename the migraiton and resolve the conflict. Then we can merge. |
github-actions
Bot
added
review/awaiting-author
The sidebar order came straight from `listCollections`, which sorted by slug. A site's collections then appear in an order nobody chose — "Certifications, Education, Endorsements, Pages, Positions, Posts, Projects" — with no way out short of renaming slugs (breaking URLs and queries) or forking the admin. Adds `sort_order` on `_emdash_collections`, settable by dragging rows on the Content Types screen or via `sortOrder` in a seed file. The column is nullable rather than defaulting to 0: NULL means "no explicit position", and those collections keep the alphabetical order behind the ordered ones, so a site that never reorders renders exactly as before. Reads materialise the fallback with COALESCE instead of relying on NULL ordering, which SQLite puts first and Postgres last on ASC. `reorderCollections` takes the full desired order and clears the position of anything left out, so the stored state stays a faithful picture of what the admin renders instead of a sparse set the UI has to reconcile. `reorder` becomes a reserved collection slug: the static POST /schema/collections/reorder route would otherwise shadow a collection by that name. Same defence in depth already applied to byline-fields/reorder. Closes emdash-cms#474 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pre-empts the review notes already raised on the sibling `hidden` PR. The full schema export the CLI reads for `emdash types` builds its collection shape by hand and omitted the new field. The comment changes apply AGENTS.md: drop justification and narrative, keep the non-obvious constraints (route ordering, NULL sorting across dialects), and remove issue references from test titles. The migration docstring also pointed at a symbol name that never existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
616ef4e to
ae7c280
Compare
|
Rebased this branch over main now that #2264 has landed (maintainer edit), resolving the overlap between ~ 🤖 Fable 5 |
ascorbic
added
bot:review
github-actions
Bot
added
review/needs-rereview
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for the problem: it adds a nullable sort_order column, materialises the fallback with COALESCE so SQLite and Postgres agree, exposes drag-and-drop reordering in the Content Types UI, and reserves reorder as a slug to avoid the new static route shadowing collection CRUD. The implementation follows EmDash's conventions — handler returns ApiResult, route uses requirePerm("schema:manage") + unwrapResult, the admin uses Lingui and invalidates the manifest query, and tests cover ordering, seed round-trips, unknown/duplicate rejection, and the reserved slug.
I read the full diff, the changed files, the manifest/sidebar consumers, the route injection order, and the registry tests. I found no logic bugs, security issues, AGENTS.md convention violations in SQL/auth/API shape, or missing locale filters. The only issues are two comment-discipline notes: both comments restate what the adjacent code already says and should be removed.
Headline conclusion: clean, well-tested feature. Minor comment cleanup only.
|
|
||
| // Optimistic order: the drop lands immediately, the server order takes | ||
| // over once the mutation invalidates the query. | ||
| const [order, setOrder] = React.useState<string[] | null>(null); |
There was a problem hiding this comment.
[suggestion] This block restates the optimistic-update behavior that is already expressed by setOrder(next) and onReorder?.(next) on lines 81–82. Per AGENTS.md comment discipline, comments should not narrate what the code plainly does.
| // Optimistic order: the drop lands immediately, the server order takes | |
| // over once the mutation invalidates the query. | |
| const [order, setOrder] = React.useState<string[] | null>(null); | |
| const [order, setOrder] = React.useState<string[] | null>(null); | |
| const serverOrder = React.useMemo(() => collections.map((c) => c.slug), [collections]); | |
| const orderedSlugs = order ?? serverOrder; | |
| React.useEffect(() => { | |
| setOrder(null); | |
| }, [serverOrder]); |
| /** | ||
| * Collection ordering shared by every list read: explicit `sort_order` | ||
| * first (ascending), then alphabetically by slug. | ||
| */ | ||
| const collectionOrder = sql<number>`coalesce(sort_order, ${sql.lit(UNORDERED_COLLECTION_RANK)})`; |
There was a problem hiding this comment.
[suggestion] The constant name collectionOrder and its definition already make it clear that this expression orders collections by sort_order with a fallback. The JSDoc repeats the same information and can be deleted.
| /** | |
| * Collection ordering shared by every list read: explicit `sort_order` | |
| * first (ascending), then alphabetically by slug. | |
| */ | |
| const collectionOrder = sql<number>`coalesce(sort_order, ${sql.lit(UNORDERED_COLLECTION_RANK)})`; | |
| const collectionOrder = sql<number>`coalesce(sort_order, ${sql.lit(UNORDERED_COLLECTION_RANK)})`; |
2 participants
What does this PR do?
Lets a site choose the order its collections appear in the admin sidebar, instead of always sorting alphabetically by slug.
Today
listCollectionssorts by slug, and that order flows through the manifest into the sidebar. The issue's example is a portfolio site that always renders "Certifications → Education → Endorsements → Pages → Positions → Posts → Projects", with no way out short of renaming slugs (breaking URLs and queries) or forking the admin.Two ways to set it:
sortOrderin a seed file, for sites that provision their schema from code:{ "slug": "projects", "label": "Projects", "sortOrder": 1, "fields": [] }Three decisions worth flagging for review:
DEFAULT 0. NULL means "no explicit position": those collections keep the alphabetical order and are listed after the ordered ones. With a0default, pinning two collections would have left every other one sorting ahead of them, which is the opposite of what pinning means. It also makes the migration a no-op visually — a site that never reorders renders exactly as before.COALESCErather than relying on NULL ordering. SQLite sorts NULL first on ASC, Postgres sorts it last, so a bareORDER BY sort_orderwould have given D1 and Postgres sites different sidebars.packages/core/tests/integration/database/dialect-compat.test.tscovers the dialect pair generally; the ordering itself is asserted in the registry tests.reorderis now a reserved collection slug. The staticPOST /schema/collections/reorderroute is injected before the dynamic[slug]route, so a collection calledreordercould never be addressed at its own URL. Reserving it at the data layer mirrors whatbyline-fields/reorderalready does. Sites with an existing collection by that name are unaffected — the reservation only rejects new creates.reorderCollectionstakes the full desired order and clears the position of anything left out, so the stored state stays a faithful picture of what the admin renders rather than a sparse set the UI has to reconcile. Unknown and duplicate slugs are rejected rather than silently applied.On accessibility: the drag handles are real buttons with an accessible name per row (
Reorder Posts, not a bare icon), and theKeyboardSensoris wired withsortableKeyboardCoordinates, so the whole reorder is doable from the keyboard — same pattern as the widgets and repeater lists.Closes #474
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
15 new tests:
packages/core/tests/unit/schema/registry.test.ts— default is unordered; ordered collections come first and the rest stay alphabetical; the same order applies on thelistCollectionsWithFields(manifest) path;reorderCollectionsassigns positions, clears omitted ones, and rejects unknown/duplicate slugs without touching the order; update preserves the position whensortOrderis omitted and clears it onnull;reorderis rejected as a slug.packages/core/tests/unit/seed/apply.test.ts—sortOrderis applied from a seed and drives the resulting order.packages/admin/tests/components/ContentTypeList.test.tsx— a labelled drag handle per row when reordering is enabled, none without it or for a single collection, rows render in the given order, plus the puremoveCollectionreducer (down, up, and the no-op cases that must skip the network call).Two notes on that output, both checked against a clean
mainby stashing this branch:createVirtualModulesPlugin scheduler wiring watches resolved sandbox plugin entries) reproduces on unmodifiedmainon this machine — pre-existing, unrelated to this change.SeoPanel.test.tsx("expected 1 times, got 2"). It passes in isolation and passed on a repeat full run (1249/1249);SeoPanelis untouched here. Flagging it as a flake I saw rather than leaving it out.