Uh oh!
There was an error while loading. Please reload this page.
Updated media browser with fresco new version - #550
Merged
Conversation
Etcher 0.3 dropped the per-op events (etcher:created/updated/deleted/ selected) plus the Etcher.Storage behaviour. Annotations now live inside the host <Fresco.canvas>'s extensions map; the library emits one bulk etcher:annotations-changed event and stays out of the consumer's DB. Changes: - media_browser.html.heex: <Fresco.viewer> → <Fresco.canvas canvas=...>, dropped <Tessera.layer> (broken against Fresco 0.5), pruned the obsolete target_type / target_uuid / initial_annotations attrs from <Etcher.layer>. - media_browser.ex: replaced the four per-op handlers with a single etcher:annotations-changed handler that diffs the bulk payload against the last-known state and dispatches create/update/delete via the adapter, then reloads from DB so comment-derived metadata stays fresh. Builds the Fresco.Canvas struct per opened file with extensions.etcher seeded. - etcher_adapter.ex: stripped @behaviour Etcher.Storage and the @impl annotations — module survives as a plain helper called from the LV. - annotation.ex: added :uuid to @cast_fields. Etcher 0.3 generates UUIDv7 client-side at draw time so the server doesn't reassign; the schema's autogenerate: true only fires when the cast leaves :uuid unset. Without this, server-side INSERTs minted a fresh uuid and the JS-side patch-shape bridge silently no-op'd (client layer had no shape under the server uuid), leaving newly-drawn shapes with bare "Rectangle"/"Polygon" tooltips. - phoenix_kit.js: added a phx:etcher:patch-shape bridge that forwards server-pushed metadata patches into the canvas's layer.patchShape API. Needed because phx-update="ignore" on <Fresco.canvas> freezes the data-extensions attr after mount — live metadata updates can't ride along on a re-render, they have to mutate the in-memory shape directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Etcher 0.3 bulk annotations-changed event doesn't carry the per-draw anchor coords the old per-op etcher:created event did, so the server can no longer seed container-space left/top for the composer popover — it was falling back to top:16, left:16. AnnotationComposerPosition hook now finds the shape via [data-uuid="<uuid>"] (Etcher tags each shape's root SVG with the annotation uuid) and positions the popover centered above the shape, flipping below when there's no room above and clamping into the container. Self-positioning sidesteps the need for any image-to-screen math server-side and keeps the popover correct after pan/zoom. The composer_anchor socket assign was nil at every assignment site after the migration, so it's removed entirely along with its style plumbing. Inline style is now just "left:0;top:0" as a hook-driven placeholder.
Pre-fix, sync_annotations chose a composer target from the annotations-changed diff: any uuid that was new in the batch (and not text-kind) opened the composer. That fired on undo-of-delete because _recreateFromSnapshot emits annotations-changed too — the restored shape was "new" relative to the post-delete state, so the composer ambushed the user mid-undo. Etcher now emits a dedicated etcher:shape-drawn event from _finalizeShape only on real user draws. The new handler opens the composer for non-text kinds (text shapes use Etcher's inline editor) and respects an already-open composer (mid-compose draws don't hijack focus). The pick_composer_target helper is gone — composer opening is no longer a function of the bulk diff.
Soft-delete (status: "deleted") left annotation comments visible in the file's thread sidebar as [removed] placeholders after the user erased the shape — confusing, since the shape and its discussion are conceptually a single unit. Switched the cascade to a real delete: matching rows are removed from phoenix_kit_comments outright, the FK ON DELETE CASCADE on phoenix_kit_comment_media drops the attachment links, and the underlying media files (referenced via comment_media.file_uuid) stay in the library. Removed Annotations.restore_linked_comments/3 — its only purpose was re-publishing soft-deleted comments on undo-of-delete, which has no soft-deleted rows to find anymore. Was already unused (Etcher 0.3's bulk annotations-changed event doesn't carry restore intent), so this just cleans up dead code. If undo-restore of comments is wanted back, the design needs an explicit client-side snapshot + restore RPC instead.
V120 unconditionally ran CREATE INDEX IF NOT EXISTS on phoenix_kit_doc_template_presets, which aborts with undefined_table on parent apps that never installed the Document Creator module (the presets table is created by that module's own bootstrap, not core PhoenixKit). The whole V120→V121 batch then rolled back, leaving the DB stuck at V119. Wrapped the CREATE INDEX in a DO $$ ... IF EXISTS information_schema ... END $$ guard matching the pattern already used for the column drop above it. Hosts with the table get the index; hosts without it skip cleanly. Idempotent either way.
Schema side of the new Etcher line tool — pairs with the JS in the sibling etcher repo. - V121 migration widens phoenix_kit_annotations_kind_check to accept 'line' (same shape as V119's dimension addition). - @kinds in Annotation schema gains "line"; @current_version pointer bumps to 121 so PhoenixKit.Migration.up runs the new migration on next consumer migrate. - MediaBrowser's <Etcher.layer tools=...> list gains :line. Also: sync_annotations now logs failed inserts / updates / deletes via Logger.warning. The previous `_ = EtcherAdapter.create(...)` swallowed every error — when a new kind was added but the DB CHECK hadn't been migrated yet, every INSERT failed silently and the only symptom was "tooltip shows the kind name and no title sibling renders" with no log to point at the root cause. Now the failure includes kind + uuid + reason so the diagnosis is one log line away.
When the user draws a shape and the composer opens, hitting Cancel used to just close the popover and leave the shape on the canvas as an untitled placeholder. The annotation was never finished — the shape was effectively orphaned. Now Cancel drops the shape so the canvas reflects the user's intent. Server side: `rollback_annotation_compose` (the path called when the composer's send_update arrives with `action: :annotation_composer_cancelled`) now pushes a new `etcher:delete-shape` LV event keyed on `fresco_id + uuid`. Client side: a new bridge in phoenix_kit.js listens for `phx:etcher:delete-shape` and calls `layer.deleteShape(uuid)` on Etcher's layer. That removes from Etcher's self.shapes + DOM, pushes the deletion onto Etcher's undo stack (Cmd+Z restores if the user changes their mind), and fires `annotations-changed` — which the existing sync handler picks up to delete the DB row and cascade the comment hard-delete. The passive `rollback_pending_annotation_if_any` path (called when the modal closes or the user switches files) stays untouched: those aren't an explicit "I don't want this shape" signal, so the annotation persists as an untitled row that the user can edit later.
… wrapper
Fresco 0.5.0 and Etcher 0.3.0 shipped to hex, so the local path-dep
detour is over. Three coordinated changes that close the loop on the
MediaBrowser → Etcher 0.3 migration:
1. mix.exs: {:fresco, path: "../fresco", override: true} →
{:fresco, "~> 0.5"}; same for etcher → "~> 0.3". Dropped
`override: true` (no transitive resolution to override anymore)
and rewrote the comment to describe the actual 0.5/0.3 surface
instead of the OSD-era 0.1/0.2 one. mix.lock follows.
2. phoenix_kit.js CDN URLs bumped: fresco@v0.1.5 → v0.5.0 and
etcher@v0.2.6 → v0.3.0. jsDelivr resolves these `gh/<user>/<repo>@<tag>`
refs from the published GitHub release tags.
3. New PhoenixKitHooks.FrescoCanvas lazy-load wrapper alongside the
existing FrescoViewer one. MediaBrowser renders <Fresco.canvas>
(phx-hook="FrescoCanvas") for annotation hosting; without this
wrapper, parent apps relying on PhoenixKit's CDN load (rather than
importing fresco.js directly) would mount <Fresco.canvas> with no
hook attached and the canvas would never initialize. Mirrors the
FrescoViewer wrapper exactly — one fresco.js bundle exports both
hooks, so a single CDN fetch covers either or both.
Closes the Fresco 0.5 / Etcher 0.3 migration arc. The local path-dep
testing scaffolding (override:true, ../fresco / ../etcher) is gone;
phoenix_kit now resolves both libraries from hex like any other dep.…iaViewer
The canvas + annotations + composer + comments-thread stack used to
live inline inside MediaBrowser (~360 LOC of helpers + heex), and
MediaViewer (timujinne's lightbox) rendered flat images via
<.image_set> with no annotation surface. Admins opening files via
MediaGallery → MediaViewer got a downgraded experience — no pan/zoom,
no Etcher overlay, no comments.
This refactor lifts the per-file viewer responsibility into a shared
`PhoenixKitWeb.Components.MediaCanvasViewer` LiveComponent that both
parents embed. Owns:
- <Fresco.canvas> + <Etcher.layer> for images; video / PDF / icon
fallback for other types
- The composer popover (with AnnotationComposerPosition hook)
- Sidebar: filename, Download, type/MIME/size/uploaded-at, comments
thread (CommentsComponent keyed on file uuid)
- Close button + prev/next chevrons (rendered inside, events routed
back to the parent LC via phx-target={"##{@parent_id}"} since the
parent owns modal open/close + file navigation)
- `viewer_canvas` / `viewer_annotations` / `composing_annotation_uuid`
assigns
- `etcher:annotations-changed` + `etcher:shape-drawn` event handlers
- Composer Post / Cancel update/2 paths (delete-shape JS push on
Cancel, patch-shape on Post; cascade-delete linked comments on
annotation delete)
API:
<.live_component
module={PhoenixKitWeb.Components.MediaCanvasViewer}
id={"media-canvas-viewer-" <> file.file_uuid}
file={file}
current_user={@phoenix_kit_current_user}
parent_id={@id}
has_prev={has_prev}
has_next={has_next}
/>
The id encodes the file uuid so prev/next navigation destroys the LC
and mounts a fresh one (Fresco.canvas's phx-update="ignore" otherwise
freezes the canvas DOM after mount).
### MediaBrowser changes
- Deleted ~360 LOC of moved helpers + assigns + event handlers + the
composer update/2 clauses
- Replaced the inline two-column viewer block with the new embed
- Kept: `viewer_file`, `step_viewer`, `close_viewer`, `viewer_keydown`,
the ViewerKeydown hook, and the modal shell + backdrop
### MediaViewer (timujinne's lightbox) changes
- Dropped <.image_set> + the variants_map / file_structs resolve
helpers (those were image-only, no annotation surface)
- Added `current_user` attr so the composer + comments thread render
for admin users
- Added a slim `curate_file/1` that resolves to MediaCanvasViewer's
expected curated-map shape (parallel to MediaBrowser's enrich_files
minus folder_path; lightbox doesn't show breadcrumbs)
- Kept the <dialog> shell + Escape-via-MediaViewerDialog hook +
navigation state
### AnnotationComposer change
The 3 hardcoded `send_update(MediaBrowser, ...)` call sites are now
`send_update(parent_module, ...)` driven by a new `parent_module`
attr. MediaCanvasViewer passes `parent_module={__MODULE__}` so
composer lifecycle actions land on the child LC.
### MediaGallery (parent of MediaViewer)
Dropped the now-unused `variants_map` / `file_structs` from the
MediaViewer embed; threads `current_user` through so the composer
+ comments work for admins.
### Single-root constraint regression test
Phoenix LiveView raises ArgumentError at render time if a stateful
component's template has anything before the root tag (including
`<% %>` expressions or HEEx comments). `rendered_to_string/1`
doesn't surface this — only inspecting the Rendered struct directly
does. Added `media_canvas_viewer_test.exs` with a single-root
assertion to catch this without needing to trigger the runtime
crash.
The MediaViewer test file was updated to drop assertions on the
moved-away surface (chevrons, download link, variants_map) and now
focuses on what MediaViewer still owns: navigation state, event
routing, single-root.ddon pushed a commit
that referenced
this pull request
May 18, 2026
Post-merge review of PR #550 surfaced three backend issues: - V121's pg_constraint existence guard checks conname globally, but constraint names are unique per namespace — on a multi-prefix install it matches another prefix's identically-named constraint and skips the ADD, leaving this prefix's phoenix_kit_annotations with no kind check. Drop the DO-block guard; the DROP CONSTRAINT IF EXISTS right before each ADD already makes the re-add unconditional and safe. - Annotation.changeset casts :uuid (needed so client-generated UUIDv7s survive INSERT), but Annotations.update/2 ran the same changeset on the update path — a stray payload uuid could rewrite the primary key. Strip :uuid from attrs in update/2; the row is already located by it. - MediaCanvasViewer.sync_annotations/3 issued an UPDATE for every existing annotation on every etcher:annotations-changed event (Etcher re-broadcasts the full list on any mutation), so a file with N annotations cost N writes per interaction. Add an annotation_unchanged? dirty-check on geometry/style/kind so untouched rows skip the UPDATE. Remaining items (linked-comment hard-delete reply handling, creator_uuid whitelist) left for the developer — see the review doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon pushed a commit
that referenced
this pull request
May 18, 2026
Follow-up cleanup from a /simplify pass over the post-merge review fixes: - Activity: extract the duplicated four-filter "any active?" check (it was inline in both the toolbar Clear button and the empty-state message) into a single any_filter_active?/1 helper. - MediaCanvasViewer.sync_annotations/3: gate the post-loop DB reload + canvas rebuild behind a wrote?/to_delete check, so an Etcher re-broadcast with no net change does no DB work at all. The main loop is now an Enum.reduce tracking whether any row was persisted. Review docs updated with resolution status for both PRs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon pushed a commit
that referenced
this pull request
May 18, 2026
Bump @Version and add the 1.7.113 CHANGELOG section covering PR #549 (media picker subfolder fix, MediaGallery refactor, filter relocation, search regression fix), PR #550 (Fresco 0.5 / Etcher 0.3 migration, MediaCanvasViewer, line annotation kind + V121), and the post-merge review fixes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
ddon pushed a commit
that referenced
this pull request
May 25, 2026
…elist + FOLLOW_UP.md The NITPICK from CLAUDE_REVIEW.md was the only still-live finding — `creator_uuid` was castable from adapter event payloads even though the adapter resolves it server-side from actor opts (a forged payload could claim authorship). Mirroring the existing `:file_uuid` exclusion. Other findings (V121 constraint guard, `:uuid` castable on update, re-UPDATE of unchanged annotations) all fixed pre-existing in commit 5198eb3. Hard-delete of linked comments is intentional per moduledoc. See FOLLOW_UP.md for the full triage.
ddon pushed a commit
that referenced
this pull request
May 25, 2026
Release rollup since 1.7.120: - PR #568: native <dialog> modal (PkDialog), core list-UI toolkit (BulkSelect, Sortable, ReorderModal, load_more), race-free sort_selector - PR #568 post-merge review fixes (untranslated reorder label, named group/row) - PR #569: PhoenixKit.boot/1 hook, locale-aware Activity dates, broad i18n sweep - PR #550/#552/#554/#557/#558/#559 follow-ups Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.