refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

refactor(media): unify file and media previews across clients - #9253

Merged
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers
Sep 3, 2026
Merged

refactor(media): unify file and media previews across clients#9253
juliusmarminge merged 8 commits into
mainfrom
t3code/mobile-media-viewers

Conversation

@juliusmarminge

@juliusmarmingejuliusmarminge commented Sep 2, 2026

Copy link
Copy Markdown
Member

What changed

  • Moved markdown file-link parsing, media-source resolution, signed URL state, and media-action availability into client-runtime so web and mobile use the same rules.
  • Routed expanded web videos through the shared streaming player, including URL refresh and retry.
  • Routed every iOS video through AVKit and every iOS image, PDF, and HTML preview through Quick Look.
  • Moved media actions to a long-press on the thumbnail and removed the redundant in-player controls.

Native long-press menus on markdown file chips are in #9258.

Why

The web and mobile clients had separate path parsers, media resolvers, URL lifecycle code, and viewer paths. The copies had already drifted. Some videos downloaded before playback, expired URLs left expanded viewers stuck, and iOS media sometimes opened in React Native viewers instead of AVKit or Quick Look.

This puts the shared decisions in one package and leaves each client responsible only for presentation. On iOS, native viewers own media playback and document preview.

UI changes

Before: clipped initial video controlsAfter: AVKit controls fill the thumbnail

Pre-fix tap and long-press race reproduced on device: screen recording

Testing

  • 40 focused mobile file, markdown, and media tests
  • Mobile TypeScript typecheck
  • TypeScript lint and formatting checks for changed files
  • Native Swift and Kotlin static checks
  • Physical iPhone 16 Pro pass against a disposable LAN environment, covering workspace and external files, images, videos, HTML, PDFs, attachments, composer drafts, markdown embeds, thumbnail long-press menus, AVKit, and Quick Look

Checklist

Created with gpt-5.6-sol using the Codex harness in T3 Code.

Note

Unify file and media preview parsing and playback across web and mobile

  • Adds shared markdown file-link and media-source resolvers in client-runtime (parseMarkdownFileLink, resolveMediaSource) and replaces local parsing implementations in web, mobile, and the t3-markdown module with these shared helpers
  • Reworks video previews on both platforms to use the shared MediaVideoPlayer; mobile removes the Expo-based VideoPlayback and MediaVideoPreviewModal, and web replaces raw <video> rendering in ExpandedImageDialog, MessagesTimeline, and ChatMarkdown with the shared player
  • Moves AssetUrlState and assetUrlStateFromResult into client-runtime; web and mobile asset URL hooks now derive loading/failure/success through the shared mapper instead of local inline checks
  • Unifies media action menus via a shared MediaActionId union and MediaActionsMenu/useMediaActions; both platforms now expose save/share/open-file actions through the same identifiers and long-press surfaces
  • Risk: resolveViewedImageAsset and ViewedImageAsset now only return media-file resources, removing the attachment-resource variant — callers expecting attachment resources from viewed workspace images will break; iOS FilePreview always uses NativeFilePreview and no longer renders MediaImagePreview for actionable images; MediaVideoPlayer no longer renders an expand/maximize control or accepts an onExpand callback

Macroscope summarized 452f18a.


Note

Medium Risk
Broad cross-platform media and preview behavior changes (iOS native viewers, inline vs modal video, shared URL lifecycle) touch high-traffic chat and file flows, though logic is consolidated and covered by focused tests.

Overview
Centralizes markdown file-link parsing, media-source classification, signed asset URL state, and media action IDs in client-runtime, then rewires web and mobile to call those helpers instead of duplicated local heuristics.

Mobile drops bespoke link/path code in favor of parseMarkdownFileLink and resolveMediaSource. Video and attachment UX converges on useMediaActions / MediaActionsMenu (long-press on thumbnails); VideoAttachmentMenu, MediaVideoPreviewModal, and the workspace image prefetch atom are removed. iOS routes all full-screen file previews through Quick Look and full-screen videos through AVKit (with URL re-mint on failure); inline/workspace video stays on Expo Video. Composer and thread attachments share attachmentVideoPreviewSource and attachment-scoped asset resources.

Web plays timeline and markdown videos inline via the shared MediaVideoPlayer (no separate “open video” flow or expand control); expanded dialogs stream through the same player with asset refresh. File attachment clicks no longer auto-open a modal for videos. Context menus align action IDs (save, copy-full-path, etc.) and expose save for videos as well as images.

Docs are updated to describe streaming, long-press save/share, and platform-specific players.

Reviewed by Cursor Bugbot for commit 452f18a. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actionsgithub-actionsBot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). 📱 Native Change Changes the native fingerprint; merging blocks production OTAs until a new store build ships. labels Sep 2, 2026
@github-actions

github-actionsBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire13.3 KiB13.5 KiB+152 B (+1.1%)15.1 KiB
CodexThread snapshot wire6.9 KiB6.9 KiB+6 B (+0.1%)7.3 KiB
CodexLive turn WebSocket wire6.4 KiB6.6 KiB+146 B (+2.2%)7.8 KiB
CodexLive turn WebSocket decoded55.6 KiB57.0 KiB+1.4 KiB (+2.6%)66.4 KiB
CodexLive turn messages10100 (0.0%)21
ClaudeTotal thread wire13.3 KiB13.3 KiB+27 B (+0.2%)15.1 KiB
ClaudeThread snapshot wire6.9 KiB6.9 KiB−6 B (−0.1%)7.3 KiB
ClaudeLive turn WebSocket wire6.4 KiB6.4 KiB+33 B (+0.5%)7.8 KiB
ClaudeLive turn WebSocket decoded56.4 KiB56.4 KiB+44 B (+0.1%)66.4 KiB
ClaudeLive turn messages910+1 (+11.1%)21

Baseline: 7751299 · PR result: 452f18a · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.4 KiB
  • Claude decoded thread snapshot: 110.1 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Comment threadapps/web/src/components/media/MediaActions.tsx Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/modules/t3-markdown-text/src/SelectableMarkdownText.ios.tsx Outdated
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadapps/mobile/src/features/files/WorkspaceFileVideoPreview.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/modules/t3-markdown-text/ios/T3MarkdownText.mm
Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx
@macroscopeapp

macroscopeappBot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a cross-client media-preview redesign that changes playback, asset URL minting, sharing, file-link parsing, and action menus across shared runtime, web, and mobile production paths. The new behavior and infrastructure span many existing surfaces, exceeding the scope of an auto-approvable refactor.

Not approved because:

  • Monthly spending limit reached (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

Comment threadpackages/client-runtime/src/markdownLinks.ts
Comment threadpackages/client-runtime/src/markdownLinks.ts Outdated
Comment threaddocs/internals/mobile-navigation.md Outdated
Comment threadapps/mobile/src/components/ComposerAttachmentStrip.tsx
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/mobile/src/features/threads/ThreadFeed.tsx
Comment threadapps/mobile/src/lib/mediaActions.ts
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/components/chat/ExpandedImageDialog.tsx Outdated
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/rightPanelStore.ts
Comment threadapps/web/src/components/ChatView.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 3d7b62e to ffddf3aCompareSeptember 2, 2026 23:12
Comment threadapps/web/src/components/chat/MessagesTimeline.tsx Outdated
Comment threadapps/mobile/src/components/FilePreview.ios.tsx
Comment threadapps/mobile/src/features/files/WorkspaceFileImagePreview.tsx
Comment threadapps/mobile/src/components/MediaVideoPlayer.tsx Outdated
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 7bd208e to 8dcd45fCompareSeptember 3, 2026 00:51
Web and mobile each had their own markdown file-link parser, media
source resolver, signed URL lifecycle, and viewer paths, and the copies
had drifted. The shared decisions now live in client-runtime:
parseMarkdownFileLink, resolveMediaSource, and assetUrlStateFromResult.
Each client keeps only presentation.
Expanded web videos play through the shared MediaVideoPlayer, which
mints and refreshes attachment URLs itself. Every full-screen iOS video
preview is AVKit and every iOS image, PDF, HTML, and SVG preview goes
through Quick Look. Media actions live on a long-press of the
thumbnail, so the player carries no menu.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarmingeforce-pushed the t3code/mobile-media-viewers branch from 8dcd45f to 7896ae8CompareSeptember 3, 2026 00:52
Comment threadapps/mobile/src/components/VideoPreviewModal.tsx Outdated
Julius Marmingeand others added 3 commits September 2, 2026 18:03
…ep their lease
Markdown images passed alt text as the preview name, so Quick Look
could not tell an SVG from its extension and threw. The preview now
uses the source file name when there is one.
A draft update on the same file re-ran the local video lease and
disposed the file Android was still playing. The lease now keys on the
draft id and file uri only, and clears stale state before reloading.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failed or still-signing video tile disabled its Pressable, which also
swallowed the long-press that opens copy, open, and save. The workspace
image error overlay sat above the menu and blocked it the same way.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stale signed URL failed, the alert closed the player, and the next
open reused the same cached URL. The failure now refreshes the asset
URL in the background so the next open plays from a fresh one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@cursorcursorBot 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but a cloud agent failed to start.

Reviewed by Cursor Bugbot for commit 05b0284. Configure here.

Comment threadapps/mobile/src/components/VideoPreviewModal.ios.tsx Outdated
Julius Marmingeand others added 4 commits September 2, 2026 18:27
The refresh callback changes identity with the connection, and having it
in the presentation effect's deps dismissed and re-presented the player
on reconnect. It is now read through useEffectEvent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native controls already offer full screen; the overlay button only
opened a dialog around the same player.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The play tile opened a dialog around the same player; the tile now is
the player, with retry and the media menu, matching markdown and
workspace videos. The dialog path for sent videos had no caller left
and is removed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
export function resolvePathLinkTarget(rawPath: string, cwd: string): string {
const { path, line, column } = splitPathAndPosition(rawPath);
const position = splitFilePathPosition(rawPath);
const { path } = position;

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.

🟡 Mediumsrc/terminal-links.ts:251

Activating a POSIX path ending in :0, such as logs/output:0, resolves it to <cwd>/logs/output instead of the actual file <cwd>/logs/output:0. splitFilePathPosition drops the non-positive line suffix and formatFilePathPosition does not restore it; preserve the suffix as part of the path when no valid line is parsed.

- const { path } = position;+ const path =+ position.line === undefined && rawPath.endsWith(":0") ? rawPath : position.path;
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/terminal-links.ts around line 251:
Activating a POSIX path ending in `:0`, such as `logs/output:0`, resolves it to `<cwd>/logs/output` instead of the actual file `<cwd>/logs/output:0`. `splitFilePathPosition` drops the non-positive line suffix and `formatFilePathPosition` does not restore it; preserve the suffix as part of the path when no valid line is parsed.

uri,
unavailable: error !== null,
error,
actionsSource: uri === null ? undefined : { name: attachment.name, mimeType, uri },

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.

🟠 Highcomponents/VideoPreviewModal.tsx:94

Save/share can fail after the modal closes because useLocalPlayback exposes only the resolved uri, so useMediaActions takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft attachment through the share path (or otherwise retain it for the operation), as AttachmentPreviewFile.share does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/components/VideoPreviewModal.tsx around line 94:
Save/share can fail after the modal closes because `useLocalPlayback` exposes only the resolved `uri`, so `useMediaActions` takes the URI share path without acquiring a composer-file lease. Cleanup then releases the preview lease while the asynchronous copy is still reading the source, allowing it to be deleted; pass the draft `attachment` through the share path (or otherwise retain it for the operation), as `AttachmentPreviewFile.share` does.

@@ -2642,7 +2619,6 @@ function ChatMarkdown({
copyMarkdown={copyMarkdown}
originalUrl={originalUrl}
style={authoredSizeStyle}

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.

🟡 Mediumcomponents/ChatMarkdown.tsx:2621

Workspace-backed images rendered through ChatMarkdownAssetImage no longer open the full-size preview when clicked or activated by keyboard. The onImageExpand={imageExpand} prop was removed from ChatMarkdownVideo, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

 style={authoredSizeStyle}
+ onImageExpand={imageExpand}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/web/src/components/ChatMarkdown.tsx around line 2621:
Workspace-backed images rendered through `ChatMarkdownAssetImage` no longer open the full-size preview when clicked or activated by keyboard. The `onImageExpand={imageExpand}` prop was removed from `ChatMarkdownVideo`, so the asset renderer receives no expansion callback, while direct images still retain theirs; restore that prop for workspace media.

@juliusmarminge
juliusmarminge merged commit 922bd69 into mainSep 3, 2026
27 of 28 checks passed
@@ -1296,27 +1295,6 @@ function ChatMarkdownVideo(props: {
)}
onRetry={props.onRetry}
actionsSource={props.actionsSource}

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.

Dropping onImageExpand here (and MediaVideoPlayer's onExpand) removes the only route from a markdown video embed to the expanded dialog, while the sibling image embed still expands on click (expandableMarkdownImageProps + cursor-zoom-in, same component). Inline embeds are capped by CHAT_MARKDOWN_MEDIA_MAX_WIDTH_CLASS_NAME, so a video's only remaining enlargement is the browser's own fullscreen button — and it also loses the dialog-level media actions that images keep. docs/user/composer.md (line 100, in a paragraph that covers web, desktop, and mobile) still states "video embeds show a player with controls and an option to expand", and both platforms lost that control in this PR.

If the removal is intentional, updating that doc sentence is the smallest fix; otherwise consider keeping the expand handler for video embeds so images and videos behave the same inline.

Posted via Macroscope — UI Consistency

@juliusmarminge
juliusmarminge deleted the t3code/mobile-media-viewers branch September 3, 2026 02:37
<MediaVideoPlayer
src={src}
label={item.name}
sourceFailed={assetUrl._tag === "Failure"}

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.

Attachment videos now mint their URL inside this dialog, but sourceFailed only reflects Failure. While the environment is reconnecting (connecting/backoff, or connected with no session yet), assetEnvironment.createUrl resolves to Effect.never (client-runtime state/runtime.ts), so assetUrl stays Loading and the dialog paints the black stateClassName placeholder with no message, no retry, and no end state until the connection comes back. Before this change the same click went through downloadFileAttachment, which pre-checked readPreparedConnection and toasted "The environment is not connected.", and the mobile player added in this PR (useMediaPlayback) folds connection._tag === "None" into unavailable for exactly this case — so web is now the only surface with no disconnected end state.

Smallest fix is to treat a missing prepared connection as a failure here (needs usePreparedConnection from ../../state/session), which also gives the panel its Retry button:

constconnection=usePreparedConnection(asset?.environmentId??null);// ...sourceFailed={assetUrl._tag==="Failure"||(asset!==undefined&&connection._tag==="None")}

Keeping the connection pre-check in openFileAttachment before setExpandedImage would work too.

Posted via Macroscope — UI Consistency

Comment on lines +95 to +97
Media actions (copy path, open in file viewer, save or share) belong to the surface that
opened the video, a long-press on the thumbnail, so the player carries no
menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.

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.

🟢 Lowinternals/mobile-navigation.md:95

The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, OpenVideoPreviewModal renders MediaActionsMenu with inModal in the header and exposes a Save or share video button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

-Media actions (copy path, open in file viewer, save or share) belong to the surface that-opened the video, a long-press on the thumbnail, so the player carries no-menu. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.+On iOS, media actions (copy path, open in file viewer, save or share) belong to the+surface that opened the video, a long-press on the thumbnail, so the player carries no+menu. Android exposes `MediaActionsMenu` and a `Save or share video` button in the modal. AVKit has no retry; a failure alerts and closes, re-mints the URL in the background, and the next open plays from the fresh one.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @docs/internals/mobile-navigation.md around lines 95-97:
The documentation incorrectly says the player carries no media-action menu and that actions are available only via a long-press on the thumbnail. On Android, `OpenVideoPreviewModal` renders `MediaActionsMenu` with `inModal` in the header and exposes a `Save or share video` button, so users can access preview actions directly in the modal; please scope the no-menu/long-press behavior to iOS.

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.

This tile is labelled Play ${file.name}, but on this branch of the handler it no longer plays. onFileOpen is ChatView's openFileAttachment, which routes only html/pdf to the panel and otherwise calls downloadFileAttachment — and this PR removed that function's video branch (the one that used to setExpandedImage({ images: [{ src: url, name, type: "video" }] })). So for a rehydrated draft (file.file === null with an uploadedAttachmentId, the normal state after a reload) clicking the play glyph now silently saves the file to disk instead of opening the player. The removal of the aria-busy/"Loading…" state on these lines assumed the click still ends in the expanded dialog, and timeline videos moved to MediaVideoPlayer, so this is the only surface left on the old play path.

Smallest fix is to open the dialog here instead of downloading: build the item with src: null and actionsSource: { kind: "video", name: file.name, src: null, asset: buildAttachmentVideoAsset(file.uploadEnvironmentId ?? environmentId, { ...file, id: file.uploadedAttachmentId }) } and pass it to onExpandImageExpandedVideo already mints and plays that shape. If downloading is the intended behavior for this state, the control should say so rather than "Play".

Posted via Macroscope — UI Consistency

github-actionsBot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 3, 2026
## What's Changed
* chore(ci): only run check-run agents on vouched contributors by @juliusmarminge in pingdotgg/t3code#9298
* fix(web): stop remounting markdown on every activity delta by @juliusmarminge in pingdotgg/t3code#9306
* fix(pull-requests): keep cached PR chrome on reopen by @maria-rcks in pingdotgg/t3code#9294
* feat(environments): draw each environment as the machine it runs on by @juliusmarminge in pingdotgg/t3code#9299
* feat(web): apply and remove labels from the pull request tab by @juliusmarminge in pingdotgg/t3code#9313
* fix(sidebar): collapse settled and snoozed shelves by default by @maria-rcks in pingdotgg/t3code#9314
* refactor(media): unify file and media previews across clients by @juliusmarminge in pingdotgg/t3code#9253
* fix(chat): keep live tool labels in present tense by @maria-rcks in pingdotgg/t3code#9316
* chore: audit lint directives and move plugin allowlists into config by @juliusmarminge in pingdotgg/t3code#9300
* chore(ci): narrow the Effect conventions check-run agent by @juliusmarminge in pingdotgg/t3code#9321
* refactor(mobile): style plain views with Uniwind classes instead of the theme bridge by @juliusmarminge in pingdotgg/t3code#9322
* fix(dev): share dev servers on the loopback Vite actually binds by @juliusmarminge in pingdotgg/t3code#9324
* fix(web): line up the titlebar wordmark label and version pill by @tristanmanchester in pingdotgg/t3code#9255
* fix(web): make the diff layout toggle a persisted setting by @juliusmarminge in pingdotgg/t3code#9326
* chore: dedupe lightningcss and tailwind node bindings by @juliusmarminge in pingdotgg/t3code#9331
* chore: upgrade vite-plus to 0.3.0 by @juliusmarminge in pingdotgg/t3code#9327
* feat(web): add a file tree to the diff panel and pull request code tab by @juliusmarminge in pingdotgg/t3code#9330
* feat(web): add PageUp/PageDown chat navigation by @Yash-Singh1 in pingdotgg/t3code#9315
* fix(web): collapse PR header actions to icons when narrow by @maria-rcks in pingdotgg/t3code#9334
* fix(web): resolve Vite sourcemap and supports warnings by @juliusmarminge in pingdotgg/t3code#9343
* feat(web): choose whether links open in the default browser or in T3 Code by @juliusmarminge in pingdotgg/t3code#9339
* fix(web): add press feedback to buttons by @maria-rcks in pingdotgg/t3code#9349
* feat(web): add customizable project icons by @saphid in pingdotgg/t3code#9137
* fix(mobile): stop indented code overflowing Android chat bubbles by @Adamulek123 in pingdotgg/t3code#9347
* fix(web): let the pull request list use wide screens by @juliusmarminge in pingdotgg/t3code#9351
* feat: display native app and browser icons in work logs by @Yash-Singh1 in pingdotgg/t3code#9093
## New Contributors
* @tristanmanchester made their first contribution in pingdotgg/t3code#9255
**Full Changelog**: pingdotgg/t3code@v0.0.39-nightly.20260903.1262...v0.0.39-nightly.20260903.1265
Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.39-nightly.20260903.1265
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📱 Native ChangeChanges the native fingerprint; merging blocks production OTAs until a new store build ships.size:XXL1,000+ changed lines (additions + deletions).vouch:trustedPR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@juliusmarminge