Repository navigation
FEAT: Share image, audio, and video previews with dataset details - #3251
Open
Julio César Suástegui (juliosuas) wants to merge 10 commits into
Open
Julio César Suástegui (juliosuas) wants to merge 10 commits into
Julio César Suástegui (juliosuas) wants to merge 10 commits into
Conversation
Co-authored-by: Julio César Suástegui <juliosuas@users.noreply.github.com>
Roman Lutz (romanlutz)
left a comment
Contributor
There was a problem hiding this comment.
Three media preview issues to address:
Cross-origin recovery links open in a new tab. Retry refetches a signed display URL. Tall images stay inside the preview.
A retry replaces only that member's display URL, so other playback keeps its place. The new URL goes through the same allowlist as the first load, and a failed refresh stays on screen.
A retry can still be in flight when the example is closed or replaced. Keep that response from writing its display URL, or its failure, onto the example that is open now.
A refresh now follows the same request as the open example, so closing it, switching examples, or changing runtime aborts that request before its URL can be applied. Retrying one member no longer cancels a refresh already running for another.
This branch has not been deployed
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.
Description
Closes #2749
Dataset example details can now preview image, audio, and video members using the same component as chat. I pulled
ImageWithSpinnerandMediaWithFallbackout ofMessageList.tsxinto a sharedMediaPreview, and chat passes the same props/classes as before, so it renders the same way.In dataset details, nothing is fetched until you click "Show … preview". Then the image uses
loading="lazy", audio/video usepreload="none", nothing autoplays, and hiding or unmounting the preview pauses playback. If a load fails, you get a message with Retry and Download.Backend side: the seed detail response now includes
value_urlon each member. It comes fromresolve_client_media_url_async, a thin public wrapper over the existing_resolve_and_sign_media_async, so local files go through/api/mediaand blob URLs get signed the same way as in chat.valuestays the stable stored reference andvalue_urlis just the display URL. Only the detail endpoint resolves URLs. The list endpoint doesn't touch them, so browsing the table never signs or downloads anything.On the frontend,
mapMediaUrlinutils/media.tsdecides whether a member is a preview, download-only, missing, or not media. It never turns a bare filesystem path into a browser URL, it uses the same inline allowlist asroutes/media.py, and SVG/HTML/other active types are never rendered.A few things I wasn't sure about, happy to change any of them:
binary_pathmembers are download-only, no preview./api/mediait's still downloadable since the route serves it as an attachment.value_url(that's what the existing resolver does) and the UI shows "File is missing." Returningnullmight be cleaner.preload="none", a broken file or an unsupported codec in an allowed container only fails once you hit play. Expired and inaccessible links share one generic error message for now.Screenshots
Dataset detail with media members
Image preview
Audio preview
Video preview
Tests and Documentation
MediaPreviewandmapMediaUrl, plus the dataset detail).MessageList.test.tsxis unchanged and passes 63/63, same as on main.tests/unit/infraflaked while the machine was loaded; that folder passes 398/398 on its own, same as main, and the PR doesn't touch it.)@seededagainst the real backend: 88/88. No retries.datasets.spec.tsrepeated 20x with--fail-on-flaky-tests: 200/200, and@seeded20x: 40/40 (before merging main). It checks there are no/api/mediarequests before you click, no autoplay, and that broken/SVG members don't break the table.