feat(media): add flat folder UI - #2586
Conversation
Scope checkThis PR changes 4,314 lines across 34 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. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | d2c7bfa | Aug 27 2026, 08:36 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 5165284 | Aug 26 2026, 05:11 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | d2c7bfa | Aug 27 2026, 08:36 AM |
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. |
🦋 Changeset detectedLatest commit: d2c7bfa 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 |
@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-moderation
@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.
This PR ships a solid flat-folder admin UI on top of the #2584 API foundation: bounded folder list/search, direct-folder URLs, create/rename/delete dialogs, a Location control in Media Details, keyboard focus handling, and RTL-aware folder names. The API surfaces, authorization checks, OpenAPI schemas, client types, and admin API functions look correct and consistent with AGENTS.md conventions (CSRF, envelope shape, SQL-safe LIKE/parameterization, Kumo components, Lingui strings, logical Tailwind).
However, the PR description and the actual diff contradict each other on drag-and-drop: the description lists drag-and-drop as intentionally excluded, while the code, changeset, and Media Library guide all implement and advertise drag-to-folder. That scope inconsistency is the most important thing to resolve before merge. I also found two concrete UI correctness issues (success move feedback is visually hidden; folder edit buttons ignore the canManageFolders permission flag) and a handful of smaller maintainability items. None of these are security or data-loss blockers, so the review is comment rather than request_changes while the scope/doc/code alignment is worked out.
Findings
-
[needs fixing]
packages/admin/src/components/MediaLibrary.tsx:27The PR description explicitly says drag-and-drop is "intentionally excluded," but this import (and the
DndContextusage around line 725) adds a full@dnd-kit/coredrag-to-folder interaction. The same feature is advertised in the changeset and user guide. Pick one source of truth: if drag is out of scope, remove the dnd-kit code, theonMoveMedia/can-drag wiring, and the related tests; if it is in scope, update the PR description so reviewers and maintainers know what is being merged. -
[needs fixing]
.changeset/media-library-folder-ui.md:6The changeset advertises moving media "by dragging a media card or row onto a visible folder," which conflicts with the PR description's claim that drag-and-drop is intentionally excluded. A changeset is public documentation copied into the CHANGELOG, so its scope must match the PR it describes. Reconcile this with the implementation before the release note is published.
-
[needs fixing]
docs/src/content/docs/guides/media-library.mdx:167The user guide tells readers to move files by dragging grid cards or list rows onto folders, but the PR description says drag-and-drop is excluded. The shipped docs should match both the code and the PR scope. Update this section once the drag-to-folder decision is finalized.
-
[needs fixing]
packages/admin/src/components/MediaLibrary.tsx:600The success toast for moving a file wraps its description in
sr-only, so the visible toast only shows "Moved" and hides the file name and destination folder. The failure toast right below uses a visible string description, and the design spec calls for complete names in both visible and audible feedback. Make the description visible.toastManager.add({ title: t`Moved`, description: t`File: ${item.filename}. Destination: ${folder.name}.`, variant: "success", timeout: 3000, }); -
[needs fixing]
packages/admin/src/components/MediaLibrary.tsx:990MediaFolderCardandMediaFolderListItemusecanEdit={folderActionsAvailable}to decide whether to show the edit button, butfolderActionsAvailableonly checks that the callbacks are defined. It does not consult thecanManageFolderspermission flag, so a caller that passes callbacks while settingcanManageFolders={false}would still render edit actions. Gate both components with the actual permission flag.canEdit={canManageFolders && folderActionsAvailable} -
[needs fixing]
packages/admin/src/components/MediaLibrary.tsx:1178Same permission-gap as the grid folder card:
MediaFolderListItempassescanEdit={folderActionsAvailable}without checkingcanManageFolders. Folder edit actions should require both the permission flag and the supplied callbacks.canEdit={canManageFolders && folderActionsAvailable} -
[suggestion]
packages/admin/src/components/MediaFolderDialog.tsx:81Every server-side
VALIDATION_ERRORis mapped to the same "Folder name must be between 1 and 200 characters" message. That is correct today because only length validation exists, but if the backend later rejects names for other reasons (invalid characters, normalization failure), the inline error will be wrong. Surface the server-supplied message whenmutationError.messageis present, or keep a generic fallback and reserve the length copy for local validation only. -
[suggestion]
packages/admin/src/router.tsx:832MediaPagedefines local numeric constants for author/editor roles (ROLE_AUTHOR = 30,ROLE_EDITOR = 40) instead of importing the canonicalRoleenum/constants from@emdash-cms/auth. If those numeric values ever change, the folder management and media-move permission gates will drift. Use the exported auth package constants. -
[suggestion]
packages/admin/tests/components/MediaDetailPanel.test.tsx:130This test (through line ~153) asserts exact Tailwind class names like
grid-cols-1,md:p-8, and inlinepaddingstyle values. Class-string assertions are implementation details and will break on harmless refactors (different utility classes, design-token updates, Kumo changes). Rewrite against observable behavior: rendered dimensions viagetBoundingClientRect(), computed CSS values, or responsive landmark/column semantics.
github-actions
Bot
added
review/needs-rereview
8f844f0 to
b493327
Compare
|
Review follow-up:
The stack has been restacked onto current |
b493327 to
df4a2ad
Compare
khoinguyenpham04
added
bot:review
There was a problem hiding this comment.
Re-review of #2586 after the first round. The PR description now matches the code: drag-and-drop into folders is explicitly in scope, so the earlier scope/doc contradiction is resolved. The folder-edit permission gap is also fixed (folder edit actions are now gated through folderActionsAvailable, which includes canManageFolders).
The implementation remains a solid flat-folder admin UI on the #2584 API: bounded folder list/search, direct-folder URLs, create/rename/delete dialogs, ownership-aware Location control, keyboard focus handling, and RTL-aware folder names. Core API/handler additions are consistent with EmDash conventions (CSRF, envelope shape, SQL-safe LIKE, migration uses FK ON DELETE SET NULL, route authorization uses the Permissions map).
Remaining blockers before merge:
- The success toast for drag-to-folder still hides the visible description in an
sr-onlyspan, so the toast body only shows "Moved". Same fix as noted in the first review. MediaDetailPanel.test.tsxstill asserts exact Tailwind class names and inline style values (grid-cols-1,md:p-8,padding, etc.). Per AGENTS.md, those are implementation-detail assertions that will break on harmless refactors and should be rewritten against observable behavior.
Also carried forward/re-raised as suggestions: centralizing the hardcoded role constants and surfacing server-supplied validation messages instead of assuming them.
Note: The PR description still records that a folder-specific maintainer-approved Discussion has not been found; that remains a process gate before merge per AGENTS.md.
Findings
-
[needs fixing]
packages/admin/src/components/MediaLibrary.tsx:600-603The success toast for moving a file wraps its description in
sr-only, so the visible toast only shows "Moved" and hides the file name and destination folder. The failure toast right below uses a visible string description. Make the description visible.toastManager.add({ title: t`Moved`, description: t`File: ${item.filename}. Destination: ${folder.name}.`, variant: "success", timeout: 3000, }); -
[needs fixing]
packages/admin/tests/components/MediaDetailPanel.test.tsx:167-190This block asserts exact Tailwind class names and inline style values (
grid-cols-1,md:grid-cols-2,md:p-8,padding,transitionProperty, etc.). Per AGENTS.md, class-string and style-string assertions are implementation details; they will break on harmless refactors and do not prove the dialog is usable. Rewrite against observable behavior: rendered dimensions viagetBoundingClientRect(), roles/landmarks, and visible content rather than how the CSS is spelled. -
[suggestion]
packages/admin/src/components/MediaFolderDialog.tsx:84Every
VALIDATION_ERRORis mapped to the same length message. If the backend later rejects a name for another reason (invalid characters, normalization failure), the inline error will be wrong. SurfacemutationError.messagewhen it is present, or keep a generic fallback and reserve the length copy for local validation only.const fieldError = mutationError instanceof ApiResponseError ? mutationError.code === "VALIDATION_ERROR" ? (mutationError.message || t`Folder name must be between 1 and 200 characters`) : mutationError.code === "CONFLICT" ? t`A media folder with this name already exists` : null : null; -
[suggestion]
packages/admin/src/router.tsx:832-833These local numeric role constants duplicate the canonical
Roleenum values. If the auth package constants change, the folder management and media-move permission gates will drift. Use the exported@emdash-cms/authRolevalues, or at least centralize the constants in the admin package so there is one source of truth.
github-actions
Bot
added
review/awaiting-author
khoinguyenpham04
added
the
bot:review
9241cc8 to
a12d423
Compare
github-actions
Bot
added
review/needs-rereview
a12d423 to
905d25a
Compare
905d25a to
619f5ed
Compare
619f5ed to
7536852
Compare
3419048 to
791cace
Compare
791cace to
5165284
Compare
2 participants
What does this PR do?
Adds the focused admin UI for the flat Media Library folder foundation in #2584. Editors can create, rename, browse, and delete folders; permitted users can move local media through the Media Details Location control or by dragging a grid card or list row onto a visible folder; deleting a folder returns its media to the Main library without changing IDs, URLs, storage keys, or usage records.
The interface follows the stable Strapi Media Library baseline while using native Kumo components: compact folder rows, explicit link and edit actions, Back and breadcrumb navigation, one mixed folder/media table in list mode, bounded loading/retry/load-more states, responsive mobile actions, and direction-aware folder names. Folder navigation preserves relevant search, filter, provider, view, page-size, focus, and scroll state.
This is PR2 of the folders sequence and is stacked directly on #2584. It intentionally excludes nesting, counts, bulk selection or moves, upload-to-folder behavior, Media Picker changes, provider behavior, CLI commands, MCP commands, and plugin changes. Local uploads continue to enter the Main library.
Stack:
Related media roadmap Discussions:
A folder-specific maintainer-approved Discussion was not found. Code review can proceed, but this feature must not merge until the folder scope is approved.
Type of change
Checklist
messages.pofiles are includedemdashand@emdash-cms/adminminor changesetsAI-generated code disclosure
Screenshots / test output
The UI was exercised end to end at 1512×982 and 320×800 in light/dark themes, English LTR, and Arabic RTL. The browser audit covered grid/list modes, responsive 4/3/2/1 folder breakpoints, create/rename/delete, Location, global folder search, Back/Forward and direct URLs, safe deletion, delayed pagination, loading/error/retry states, focus restoration, scroll preservation, and mixed-direction long names.