Skip to content

Unify asset preview dialog with details panel and navigation#2296

Open
SajalChaplot wants to merge 13 commits into
mainfrom
ai_main_e18ddcd4bb5448b199f6
Open

Unify asset preview dialog with details panel and navigation#2296
SajalChaplot wants to merge 13 commits into
mainfrom
ai_main_e18ddcd4bb5448b199f6

Conversation

@SajalChaplot

@SajalChaplot SajalChaplot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Consolidates asset preview into a single, reusable AssetPreviewDialog component with a details panel, download action, and next/previous navigation, replacing the duplicated inline preview dialog on the AssetPickerSurface.

Problem

The library had two separate, duplicated implementations of the asset preview dialog (one used within AssetPreviewDialog and one inlined in AssetPickerSurface). Neither surfaced asset metadata (dimensions, folder, description, prompt, etc.) or provided a quick download action, forcing users to leave the preview to view details on a separate page.

https://clips.agent-native.com/share/qHpqs6Gv7Sx5?ref=clip_share

Solution

Merged the two preview implementations into a single AssetPreviewDialog component that both surfaces now share, and enhanced it with a toggleable details sidebar, a download button, and consistent navigation arrows for moving between assets without leaving the preview.

Key Changes

  • Replaced the inline preview Dialog in AssetPickerSurface with the shared AssetPreviewDialog component, removing the duplicated markup.
  • Restyled AssetPreviewDialog into a two-pane layout (media on one side, details on the other) with a header toolbar containing download, details toggle, and close actions.
  • Added downloadAsset handler using the export-asset action mutation to trigger downloads directly from the preview.
  • Added AssetPreviewDetails, PreviewField, assetPreviewCategoryLabel, formatPreviewDimensions, and previewGcd helpers to render title, status/role/type/category badges, dimensions with aspect ratio, MIME type, folder, description, and prompt.
  • Extended the Asset type with folderId, category, model, aspectRatio, durationSeconds, and metadata fields to support the new details panel.
  • Added new icons (IconDownload, IconInfoCircle) and adjusted styling of navigation arrows and image sizing to fit the new layout.
  • Added a changelog entry describing the unified asset preview.

Edit in Builder  Preview


To clone this PR locally use the Github CLI with command gh pr checkout 2296

You can tag me at @BuilderIO for anything you want me to fix or change

@builder-io-integration builder-io-integration Bot changed the title Update from the Builder.io agent Unify asset preview dialog with details panel and navigation Jul 21, 2026
@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@netlify

This comment has been minimized.

builder-io-integration[bot]

This comment was marked as outdated.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@SajalChaplot

Copy link
Copy Markdown
Contributor Author

@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable)

@netlify

This comment has been minimized.

@builder-io-integration

Copy link
Copy Markdown
Contributor

CI caught a real issue from my last push: the new assetDetail.downloadFailed toast string I added for the download-error fix failed guard:i18n-catalogs (missing in zh-TW, untranslated fallback in the other 9 locales). Added proper translations to all 10 locale catalogs in a92d63c — guard passes locally now, waiting on CI to confirm.

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builder reviewed your changes and found 2 potential issues 🟡

Review Details

Incremental Code Review Summary

The latest commits extract AssetPreviewDialog into a reusable component and wire it into the library, picker, recent drafts, and brand-kit swimlane flows. The previous review findings were addressed: starter assets now bypass export-asset and failed downloads show a toast. The shared component also preserves custom image rendering for the picker and adds source fallbacks for previews.

This remains a standard-risk UI/action integration change. Two new regressions remain. First, the shared dialog navigates directly to the raw export-asset URL instead of applying the same assetMediaUrl normalization used by the standalone asset detail route, which can break downloads under the app's mounted /assets base path. Second, GenerationResults removes its generated-candidate “View details” link without replacing that path or migrating that dialog to the shared preview, leaving users without access to the full asset detail page from that flow.

Key findings

  • 🟡 MEDIUM: Normalize exported download URLs through assetMediaUrl before navigation.
  • 🟡 MEDIUM: Preserve a route to full asset details from the generated-candidate preview.

The refactor otherwise improves reuse and keeps the existing specialized live-slot preview behavior intact.

🧪 Browser testing: Attempted full verification after confirming the dev server was healthy, but all executor batches were blocked because browser automation tools were unavailable. The affected flows remain unverified due to infrastructure.

Comment on lines +106 to +108
onSuccess: (result: any) => {
const url = result?.downloadUrl;
if (url) window.location.href = url;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Normalize exported download URLs before navigation

The shared dialog assigns result.downloadUrl directly to window.location.href, unlike the standalone asset detail route, which wraps the value with assetMediaUrl(...). When the app is mounted under /assets or the action returns a root-relative URL, the raw path can resolve incorrectly and produce a silent 404. Apply the existing URL normalization before navigating.

Fix in Builder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the standalone asset detail route already does assetMediaUrl(result.downloadUrl) ?? result.downloadUrl for exactly this reason (mounted-app base path / root-relative fallback URLs). My shared dialog dropped that wrapping. Fixed in 2748284: the export-success handler now goes through assetMediaUrl the same way.

</Link>
</Button>
) : null}
<Button

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Preserve access to generated asset details

This removes the generated-candidate preview's only View details link to /asset/:id, but GenerationPreviewDialog is still a separate component and was not migrated to the new shared dialog. Users can still save/refine/delete, but lose the route to copy the URL, inspect full metadata, or use the standalone asset actions. Retain the link or replace this dialog while preserving those actions.

Fix in Builder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this was a real functionality regression, not just scope trimming — that candidate-review dialog has no Download/Copy-URL affordance of its own, so removing the link left no way to inspect full metadata or get a shareable URL for a live-generation candidate. I over-reached by touching this dialog; it's a genuinely different flow (Save/Refine/Dismiss on unsaved candidates) than the grids this PR was standardizing, and migrating it to the shared component is a separate, larger change I don't want to rush into this PR. Restored the link in 2748284 instead of migrating.

@builder-io-integration

Copy link
Copy Markdown
Contributor

There was a problem with your request, please try again later. Error id: c9698dd944bc4a6ca275fbf05f390abc

@SajalChaplot

Copy link
Copy Markdown
Contributor Author

@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable)

@builder-io-integration

Copy link
Copy Markdown
Contributor

All checks are now passing (no fail/pending entries) and the branch remains MERGEABLE. BLOCKED is solely due to required human review approval. Let's confirm this is the only blocking reason and do one final check for any last-minute feedback.

@SajalChaplot
SajalChaplot requested review from a team and shomix and removed request for a team July 22, 2026 10:43
@SajalChaplot

Copy link
Copy Markdown
Contributor Author

@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable)

@builder-io-integration

Copy link
Copy Markdown
Contributor

There was a problem with your request, please try again later. Error id: 28ff114ea6044326b4b4c8f7a7c16da2

@shomix shomix left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some tests are failing. O/w cool stuff

@SajalChaplot

Copy link
Copy Markdown
Contributor Author

@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable)

@builder-io-integration

Copy link
Copy Markdown
Contributor

There was a problem with your request, please try again later. Error id: b98d82ab79fa43298cfa176d93140f8e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants