feat(media): add image focal points - #2624
Conversation
Scope checkThis PR changes 1,256 lines across 44 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-cache | 95bc8b1 | Aug 26 2026, 05:05 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 95bc8b1 | Aug 26 2026, 05:09 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 95bc8b1 | Aug 26 2026, 05:10 PM |
🦋 Changeset detectedLatest commit: 95bc8b1 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: |
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. |
650077a to
686f750
Compare
c885c58 to
91e344d
Compare
There was a problem hiding this comment.
This is the right change for the approved discussion: it adds image focal points end-to-end (DB column, API validation, admin editor with accessible pointer/keyboard input, and rendering via object-position) while keeping changes additive and consistent with EmDash conventions. I read the diff, AGENTS.md, the touched source files, tests, route, schema, and migration. The implementation is mostly clean — validation is thorough, the migration handles concurrent column-add races, the UI uses Lingui/Kumo and logical Tailwind, and render paths rely on stored snapshots so no extra logged-out queries are introduced.
The only substantive issue is a public-client type leak: EmDashClient.mediaSetFolder now types the PUT /media/:id response as MediaFolderAssignment, which had focal fields added to it even though that method is only about folder assignment and the endpoint actually returns the full MediaItem. That interface is misleading and should be narrowed back to folder info or replaced with MediaItem.
No security, data-integrity, localization, or query-count regressions identified.
| /** Result of assigning local media to a folder or the Main library */ | ||
| export interface MediaFolderAssignment { | ||
| id: string; | ||
| focalX: number | null; | ||
| focalY: number | null; | ||
| folderId: string | null; | ||
| } |
There was a problem hiding this comment.
[needs fixing] The MediaFolderAssignment interface is described as the “Result of assigning local media to a folder or the Main library,” but it now carries focalX/focalY. The underlying mediaSetFolder method (line 910) only sends { folderId } to PUT /_emdash/api/media/:id, and that endpoint returns the full MediaItem (which already has focalX/focalY). Adding focal-point fields to a narrow folder-assignment type is a leaky, confusing public API contract.
Either make mediaSetFolder return Promise<MediaItem> and keep MediaFolderAssignment as { id; folderId }, or remove MediaFolderAssignment entirely if nothing else uses it.
| /** Result of assigning local media to a folder or the Main library */ | |
| export interface MediaFolderAssignment { | |
| id: string; | |
| focalX: number | null; | |
| focalY: number | null; | |
| folderId: string | null; | |
| } | |
| /** Result of assigning local media to a folder or the Main library */ | |
| export interface MediaFolderAssignment { | |
| id: string; | |
| folderId: string | null; | |
| } |
and update the method signature and request generic to use MediaItem:
| /** Result of assigning local media to a folder or the Main library */ | |
| export interface MediaFolderAssignment { | |
| id: string; | |
| focalX: number | null; | |
| focalY: number | null; | |
| folderId: string | null; | |
| } | |
| /** Assign media to a folder, or return it to the Main library */ | |
| async mediaSetFolder(id: string, folderId: string | null): Promise<MediaItem> { | |
| const data = await this.request<{ item: MediaItem }>( | |
| "PUT", | |
| `/media/${encodeURIComponent(id)}`, | |
| { folderId }, | |
| ); | |
| return data.item; | |
| } |
ef67627 to
10049f2
Compare
6095569 to
8debaa8
Compare
ascorbic
left a comment
There was a problem hiding this comment.
This is good. In future I'd like to think about how we could integrate this with image providers so that cropping works, buit for now this is good.
8f8ad7e to
d2a27e6
Compare
d2a27e6 to
95bc8b1
Compare
What does this PR do?
Adds focal points for local images. Editors can set the important part of an image from the Media Details dialog, compare square, landscape, and portrait crops, and keep that subject visible in cover-cropped thumbnails, image fields, galleries, and
EmDashImage.The dialog uses native Kumo segmented tabs for Details and Edit image. Folder Location remains under Details, the crop previews sit beside the editor on desktop, and the dialog keeps a stable height while switching tabs.
This is PR 4/4 in the native media stack and is stacked directly on #2586:
Related discussion: #990
Type of change
Checklist
pnpm typecheckpasses for the affected admin and core packagespnpm lintpasses with 0 type-aware diagnosticspnpm formathas been run and the full formatting check passesmessages.pofiles are included.AI-generated code disclosure
Screenshots / test output
PostgreSQL and D1 were not available locally; SQLite and the existing migration-race fixture passed.