Skip to content

feat: add download button for attached videos #5042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
siddseethepalli merged 1 commit into from
Feb 19, 2026
Merged

feat: add download button for attached videos #5042

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion clients/macos/vellum-assistant/Features/Chat/MediaEmbeds/InlineVideoAttachmentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ struct InlineVideoAttachmentView: View {
@State private var isLoading = false
@State private var failed = false
@State private var videoAspectRatio: CGFloat = 3.0 / 4.0
@State private var isHovering = false

var body: some View {
ZStack {
ZStack(alignment: .topTrailing) {
RoundedRectangle(cornerRadius: VRadius.md)
.fill(VColor.surface)
.overlay(
Expand All @@ -39,9 +40,22 @@ struct InlineVideoAttachmentView: View {
} else {
placeholderView
}

if !failed && !isLoading && isHovering {
Button(action: saveVideo) {
Image(systemName: "arrow.down.circle.fill")
.font(.system(size: 24))
.foregroundStyle(.white)
.shadow(radius: 2)
}
.buttonStyle(.plain)
.padding(VSpacing.sm)
.accessibilityLabel("Save video")
}
}
.frame(maxWidth: 360)
.aspectRatio(videoAspectRatio, contentMode: .fit)
.onHover { isHovering = $0 }
.onDisappear {
player?.pause()
player = nil
Expand Down Expand Up @@ -162,6 +176,34 @@ struct InlineVideoAttachmentView: View {
}
}

private func saveVideo() {
let panel = NSSavePanel()
panel.nameFieldStringValue = (attachment.filename as NSString).lastPathComponent
panel.canCreateDirectories = true
guard panel.runModal() == .OK, let destURL = panel.url else { return }

if attachment.isLazyLoad {
guard let port = daemonHttpPort, let attachmentId = attachment.id.isEmpty ? nil : attachment.id else { return }
isLoading = true

@devin-ai-integration devin-ai-integration Bot Feb 19, 2026

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.

🟡 saveVideo() sets isLoading=true which hides the active video player during save

When a user is watching a lazy-loaded video and clicks the save button, saveVideo() sets isLoading = true at line 187. Because the view body uses an if/else if chain (InlineVideoAttachmentView.swift:32-42), the isLoading branch takes priority over the player/isPlaying branch, causing the video player to be replaced by a "Loading video..." spinner while the save is in progress.

Root Cause and Impact

The isLoading state is shared between two different operations: initial video loading for playback and the save/download operation. When saveVideo() sets isLoading = true at line 187, the view body at lines 34-36 shows loadingView instead of the VideoPlayerView:

} else if isLoading {
    loadingView          // <-- replaces the video player
} else if let player, isPlaying {
    VideoPlayerView(player: player)  // <-- hidden during save

This means clicking "Save video" on an already-playing lazy-loaded video will:

  1. Hide the video player and show a loading spinner
  2. Re-fetch the attachment data from the server (even though it was already fetched for playback)
  3. After save completes, the video player reappears but the visual interruption is jarring

Impact: The video playback is visually disrupted every time the user saves a lazy-loaded video. The save operation should use a separate state (e.g. isSaving) or avoid setting isLoading altogether, since the save can happen in the background without affecting the UI.

Prompt for agents 
In clients/macos/vellum-assistant/Features/Chat/MediaEmbeds/InlineVideoAttachmentView.swift, the saveVideo() function at line 187 sets isLoading = true, which causes the video player to be replaced by a loading spinner during the save operation. To fix this:

1. Add a new @State private var isSaving = false property (around line 22)
2. In saveVideo() (lines 187-199), replace all occurrences of isLoading with isSaving
3. Optionally show a subtle save progress indicator that doesn't replace the video player (e.g. overlay a small spinner on the save button area)
4. Update the save button visibility condition at line 44 to also check !isSaving if you want to hide the button during save

This ensures the save operation doesn't interfere with video playback state.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Task {
do {
let base64 = try await fetchAttachmentData(port: port, attachmentId: attachmentId)
guard let data = Data(base64Encoded: base64) else {
await MainActor.run { isLoading = false }
return
}
try data.write(to: destURL)
await MainActor.run { isLoading = false }
} catch {
await MainActor.run { isLoading = false }
}
}
} else {
guard let data = Data(base64Encoded: attachment.data) else { return }
try? data.write(to: destURL)
Comment on lines +202 to +203

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid blocking UI while saving inline video data

In the non-lazy attachment path, both Data(base64Encoded:) and data.write(to:) run synchronously in the button action, which executes on the main thread. For larger inline videos, this can freeze the chat UI until decode+disk write finishes, making the app feel hung during save. Moving this branch to a background Task (as done for lazy-load saves) would prevent user-visible stalls.

Useful? React with 👍 / 👎.

}
}

private func openInExternalPlayer() {
if attachment.isLazyLoad {
guard let port = daemonHttpPort, let attachmentId = attachment.id.isEmpty ? nil : attachment.id else { return }
Expand Down