feat(core): add group and sortOrder to collections for admin UI organization#292
feat(core): add group and sortOrder to collections for admin UI organization#292tohaitrieu wants to merge 6 commits into
Conversation
- Add TipTap table extensions (@tiptap/extension-table, table-row, table-cell, table-header) - Add table to slash commands menu (/table) - Add table button to EditorToolbar - Add TableBubbleMenu for editing tables (add/remove rows/columns, toggle header, delete) - Update prosemirrorToPortableText converter to handle tables - Update portableTextToProsemirror converter to handle tables - Add CSS styles for table editing including dark mode support
…ization - Add group and sortOrder fields to SeedCollection interface - Add group and sortOrder to CreateCollectionInput and UpdateCollectionInput - Add group and sortOrder to Collection type - Update createCollection and updateCollection to store these fields - Update mapCollectionRow to read these fields - Update listCollections to sort by sort_order - Add migration 033_collection_groups for database schema - Update CollectionTable type with new columns This allows plugins to organize their collections into groups (e.g., Trading, LMS, Community) for better admin UI navigation.
🦋 Changeset detectedLatest commit: be552fd The changes in this PR will be included in the next version bump. 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 |
|
I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
The CLA check is failing because @emdashbot[bot] is listed as a committer. Bot accounts cannot sign the CLA. Could a maintainer please configure the CLA check to ignore bot accounts, or manually override this check? |
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. |
|
Can you fix the conflicts |
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
|
Hi @tohaitrieu, thanks for this PR! I'm working on a new proposal at Discussion #1151 focused on admin sidebar grouping for collections AND plugin admin pages (after the maintainer asked me to split my previous PR #1024). Your PR (#292) already handles the collection grouping part nicely — solid migration, types, and registry updates. If discussion #1151 gets maintainer approval, I'd like to propose adopting/continuing the work from PR #292 as the foundation for collection grouping, and I'll add plugin admin page grouping on top. Would you be interested in collaborating? We can coordinate on discussion #1151 or here. Cheers! |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach judgment
The collection groups feature is sound and correctly scoped — it has an approved Discussion (#291) and touches the right layers (DB migration, registry, seed types). The sort_order + group index and the mapCollectionRow fallback defaults are the right patterns.
However, this PR bundles an entire unrelated TipTap table editor feature (~200 lines in PortableTextEditor.tsx, new dependencies, and new CSS). That violates AGENTS.md directly:
"Do not touch code outside the scope of your change. No drive-by refactors, no 'while I'm here' improvements."
The table feature also lacks a linked approved Discussion, making it an unsolicited feature per AGENTS.md. It should be removed from this branch and PRed separately if the author wants to pursue it.
Cross-cutting concerns the diff misses
The PR adds group and sortOrder to the database layer and TypeScript types, but forgot to update the API contract:
packages/core/src/api/schemas/schema.ts—createCollectionBody,updateCollectionBody, andcollectionSchemado not includegrouporsortOrder. Because the route handlers use Zod viaparseBody, API consumers cannot set or read these fields. The feature is effectively unreachable over the wire.packages/core/src/astro/routes/api/schema/index.ts— the JSON schema export endpoint also omitsgroupandsortOrder, so the CLIemdash typesand dev-server sync won't see them.
Both files need the same additions that were made to the internal DB types.
Table feature bugs
If the table code stays (it shouldn't), it has real bugs:
- Link stripping on round-trip —
convertPTBlock's table cell handler maps marks with a simple name map (strong → bold, etc.) but never resolvesmarkDefreferences. A link stored as a Portable Text markDef will be silently dropped when reloading the editor. - Peer dependency version skew —
@tiptap/extension-table@3.22.1requires@tiptap/core@^3.22.1and@tiptap/pm@^3.22.1, but the workspace pins everything else to3.20.0. This mismatch can cause runtime errors if the extension uses APIs added after 3.20.0. - Wrong dependency section — runtime TipTap extensions are placed in
devDependencies, inconsistent with every other TipTap extension inpackages/admin/package.json. - Broken button overlay positioning — the
Plusicons inside table bubble menu buttons useabsolute -left-0.5/-bottom-0.5, butBubbleButton(and the underlyingButtoncomponent) is notrelative, so the icons will be positioned against the menu container instead of the button.
Missing tests
There are no new tests for:
listCollectionsordering bysort_ordercreateCollection/updateCollectionpersistinggroupandsortOrderapplySeedforwardinggroup/sortOrderfrom seed JSON- Migration 034 up/down behavior
| import CharacterCount from "@tiptap/extension-character-count"; | ||
| import Focus from "@tiptap/extension-focus"; | ||
| import Placeholder from "@tiptap/extension-placeholder"; | ||
| import { Table } from "@tiptap/extension-table"; |
There was a problem hiding this comment.
[needs fixing] This PR is titled "add group and sortOrder to collections" but bundles an entire TipTap table editor feature. That violates AGENTS.md's scope rule:
"Do not touch code outside the scope of your change. No drive-by refactors, no 'while I'm here' improvements."
The table feature also lacks a linked approved Discussion. Please remove all table-related changes from this branch and open a separate PR if you want to pursue tables.
| }; | ||
|
|
||
| const rows = (tableBlock.rows || []).map((row, rowIndex) => { | ||
| const cells = row.cells.map((cell) => { |
There was a problem hiding this comment.
[needs fixing] This mark mapping is naive and breaks links (and any other mark-def-based marks) inside table cells.
PortableText stores links as markDefs referenced by _key. If a span has marks: ["someLinkKey"], this code passes "someLinkKey" directly as a ProseMirror mark type, which does not exist. The link is silently stripped on round-trip.
The correct fix is to reuse the existing convertInlineContent helper (which already handles markDefs properly) instead of hand-rolling a second inline converter for table cells:
| const cells = row.cells.map((cell) => { | |
| // Map PortableText marks to ProseMirror marks | |
| const { children, markDefs } = convertInlineContent(cell.content); | |
| return { | |
| type: cellType, | |
| content: [ | |
| { | |
| type: "paragraph", | |
| content: children.length > 0 ? children : undefined, | |
| }, | |
| ], | |
| }; |
(You would also need to thread markDefs upward to convertPTBlock the same way the existing block handlers do.)
| "@tailwindcss/cli": "^4.1.10", | ||
| "@tailwindcss/typography": "^0.5.19", | ||
| "@testing-library/react": "^16.3.0", | ||
| "@tiptap/extension-table": "^3.22.1", |
There was a problem hiding this comment.
[needs fixing] These are runtime dependencies used by PortableTextEditor.tsx, but they are placed in devDependencies. Every other TipTap extension in this package lives in dependencies. Move them to the correct section.
Additionally, @tiptap/extension-table@3.22.1 declares peer dependencies on @tiptap/core@^3.22.1 and @tiptap/pm@^3.22.1, but the workspace pins all other TipTap packages to 3.20.0. That peer mismatch can cause runtime incompatibilities. Either upgrade all TipTap packages together, or pin the table extensions to the same version the rest of the monorepo uses.
| title="Add column before" | ||
| > | ||
| <Columns className="h-4 w-4" /> | ||
| <Plus className="h-2 w-2 absolute -left-0.5" /> |
There was a problem hiding this comment.
[suggestion] The Plus icon uses absolute -left-0.5, but BubbleButton (and the underlying Button component) does not set relative. The icon will be positioned against the nearest positioned ancestor — likely the BubbleMenu flex container — rather than the button itself. Add relative to the BubbleButton wrapper (or to the Button's className) so the overlay lines up correctly.
The same issue affects the other Plus icons at -right-0.5, -top-0.5, and -bottom-0.5.
github-actions
Bot
added
review/awaiting-author
3 participants
What does this PR do?
Add collection grouping support for better plugin organization in the admin sidebar. Plugins can define
groupandsortOrderin their seed.json to organize collections into logical groups (e.g., "Trading", "LMS", "Community").Discussion: #291
Changes:
group(TEXT) andsort_order(INTEGER) columns to_emdash_collectionsType of change
Checklist
pnpm typecheckpassespnpm --silent lint:json | jq '.diagnostics | length'returns 0pnpm testpasses (or targeted tests for my change)pnpm formathas been run