Fix media picker circle, selector route shadowing, and a media-surface bug sweep - #645
Merged
Conversation
The selected-state checkmark badge sized itself from padding around an inline-block icon span, so the inherited line-height strut stretched it taller than wide (28x36 in the picker modal). Give it the same fixed square as the unselected ring plus flex centering, so both states render identical true circles.
Route order: /admin/media/selector was declared after /admin/media/:file_uuid,
so the selector page was unreachable (every request cast "selector" as a UUID
and crashed).
MediaSelector page: parse ?page= safely (no crash on non-numeric values, no
negative OFFSET), guard return_to to local paths, exclude trashed and
system-managed files from the picker (parity with the modal), paginate via
patch instead of navigate so selection and search survive paging, and make
uploads refresh the grid, auto-select the new file, and flash on failure —
the {:error, _} return also crashed consume_uploaded_entry.
MediaSelectorModal: keep the selection as an ordered list instead of a MapSet
so Confirm returns files in pick order (MediaGallery's Featured image no
longer changes on reopen+confirm); clamp seeded selections to one in :single
mode; normalize mode once (string modes crashed upload auto-select); parse
change_page payloads safely.
MediaDetail: render the not-found state for non-UUID path values instead of
raising Ecto.Query.CastError.
MediaBrowser: page trash/orphaned views locally — the old set_page swapped
normal files into the trash view (uncontrolled) or exited it entirely
(controlled) — and route all local paging through reload_current_page, which
now clamps out-of-range pages after deletions; keep sort/type filters across
search, clear-search, folder navigation and the orphan toggle; clear the
carried selection when toggling trash and double-check file status before
permanent deletion (active files selected before the switch could be
destroyed); reload the active folder list when leaving trash (trashed folder
cards lingered); cover expanded-stack and cross-page selections in Select
all / Download (bulk download now resolves from the DB); make Clear only
clear instead of exiting selection mode; make folder-delete confirmations
describe the actual recursive behavior (they claimed contents move to the
parent); parse the storage_max_upload_size_mb setting defensively.
Gettext: extract and translate the new strings to et/ru.
MediaSelectorModal: new optional max_select cap (nil = unlimited) — at the cap a :multiple picker rejects further adds and shows a "Maximum N files" badge plus an "N of M selected" counter instead of letting the consumer silently truncate on confirm; seeds are clamped the same way; upload auto-select respects the cap (the file still stores and lists). MediaGallery passes its max_count through, closing the Enum.take truncation. MediaSelectorModal: the upload accept list now follows the in-modal type dropdown — re-allow the upload with the new accept when nothing is in flight (the server-side type gate still backstops in-flight changes). MediaBrowser: zero-result searches show "No files match your search" instead of the folder/library empty states, and folder cover + logo images honor the file's saved rotation — covers swap their dimensions via container-query units inside an absolutely-positioned size container, so quarter turns cover the arbitrary-aspect hero exactly; logos ride the plain square rotation transform. MediaThumbnail.normalized_rotation/1 is now public for this. Gettext: extract and translate the new strings to et/ru.
ddon
pushed a commit
that referenced
this pull request
Jul 18, 2026
…7.201 Reviewed PR #645 (media picker circle, selector route shadowing, and a media-surface bug sweep) — clean pass, no findings. Fixed an unrelated pre-existing credo --strict failure (nested-module aliasing suggestion in multilang_form_test.exs from PR #643) that was silently preventing dialyzer from running as part of mix precommit. Co-Authored-By: Claude Opus 4.8 <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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Started from an error report — the selected-image checkmark in the media picker modal rendered as an oval, not a circle — and grew into a sweep of the whole media/image surface (pickers, browser, detail page), cross-checked by a multi-reviewer pass over the same files.
The reported bug
The selected-state badge sized itself with padding around an inline-block icon span, so the inherited line-height strut stretched it to 28×36. Both selector surfaces now use the same fixed square + flex centering as their unselected ring (28×28 / 36×36, verified in-browser via
getBoundingClientRect), so both states render identical true circles.Route shadowing (found while verifying)
live "/admin/media/selector"was declared afterlive "/admin/media/:file_uuid", so the selector page was unreachable — every request boundfile_uuid="selector"and crashed withEcto.Query.CastError.MediaSelectorHelper.build_url/1exists specifically to hand out URLs to this page, all of which 400'd. Routes reordered.Sweep fixes
MediaSelector (full-page picker)
?page=abccrashed the mount (String.to_integer); zero/negative pages produced negative SQL OFFSETs → safe parse + clampreturn_to(user-controlled query param) was fed unvalidated topush_navigate→ local-path guardnavigate(full remount), wiping the in-memory selection and active search on every page change →patch"save"handler no button triggers), and a failed upload returned{:error, _}fromconsume_uploaded_entry's callback — which raises — so failures crashed the LiveView → uploads now reload, auto-select the new file, and flash on failureMediaSelectorModal (the popup)
MapSet, so Confirm returned uuid-sorted order — reopening a gallery picker and confirming silently reordered images and changed the Featured one. Selection is now an ordered list end-to-end:singlepicker seeded with multiple uuids let Confirm return all of them → clamped to onemodepassed as a string crashed the upload auto-selectcase→ normalized once inupdate/2change_pagecrashed on malformed payloads → safe parseMediaDetail
/admin/media/<x>raisedEcto.Query.CastError→ renders the existing "File Not Found" stateMediaBrowser
set_pagehad no trash arm (page 2 of Trash silently showed active files under the trash header, with permanent-delete actions), and controlled mode round-tripped through the URL contract, which resets trash — exiting the view. Trash/orphaned now page locally throughreload_current_page"trashed"(also covers a restore racing from another session)list_extra; the deadextra \\ []defaults are removed so future call sites can't silently regress thislocate_file/2reload_current_pageclampsstorage_max_upload_size_mbsetting value crashed every media surface at mount → defensive parsei18n: new/changed strings extracted; et/ru translated (other locales left for the usual catch-up).
Verified
mix precommitclean; media/storage test files 222/0,test/phoenix_kit_web363/0phoenix_kit_parent: selector page reachable and rendering;?page=abc+ externalreturn_tohandled;/admin/media/<garbage>shows File Not Found; modal open → select (28×28 circle, count badge) → Confirm round-trip works; folder kebab shows the corrected confirm textFollow-up commit: the four flagged limitations are now fixed too
MediaGallery.max_counttruncation → the modal gained an optionalmax_selectcap: at the cap a:multiplepicker rejects further adds and shows a "Maximum N files" badge plus an "N of M selected" counter; seeds clamp the same way; upload auto-select respects the cap (the file still stores and lists). The gallery passes itsmax_countthrough, so theEnum.takeon confirm is now a belt-and-braces no-opacceptlist → the modal re-allows the upload with the filter's accept whenever the type dropdown changes and nothing is in flight (the server-side type gate still backstops in-flight changes)[container-type:size]wrapper (which sizes from inset, so the content-driven hero height is unaffected) — covering the box exactly with no hardcoded scale. Logos are squareobject-containboxes and ride the plain rotation transform.MediaThumbnail.normalized_rotation/1is public now for thisVerified: precommit clean, 493 media/storage/web tests green, browser-checked the search empty state and the modal select flow (28×28 circle intact).