feat: add thumbnails to wire cells upload previews - WPB-19266 #3440
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR implements thumbnail generation for image and video assets in the Wire cells attachments carousel, replacing the placeholder nil thumbnail values with actual generated thumbnails using QuickLook.
- Adds thumbnail generation capability to
AttachmentsCarouselViewModelusing aThumbnailGeneratorprotocol - Updates the constructor to use dependency injection instead of hardcoded empty arrays
- Adds comprehensive unit tests for the thumbnail generation functionality
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ConversationInputBarViewController.swift | Updates constructor call to use parameterless initializer |
| AttachmentsCarouselViewModelTests.swift | Adds comprehensive unit tests for thumbnail generation scenarios |
| WireCellsDraft+Fixture.swift | Extends test fixture to support file type and size parameters |
| UIImage+Fixture.swift | Adds test fixture helper for creating UIImage instances |
| AttachmentsCarouselViewModel.swift | Implements core thumbnail generation logic with caching and async generation |
| AttachmentsCarouselItem.swift | Updates model to support Equatable/Sendable for testing and concurrency |
| ThumbnailGenerator.swift | Defines protocol and QLThumbnailGenerator extension for thumbnail generation |
.../WireMessagingUI/WireCells/Components/AttachmentsCarousel/AttachmentsCarouselViewModel.swift
Outdated
Show resolved
Hide resolved
.../WireMessagingUI/WireCells/Components/AttachmentsCarousel/AttachmentsCarouselViewModel.swift
Outdated
Show resolved
Hide resolved
.../WireMessagingUI/WireCells/Components/AttachmentsCarousel/AttachmentsCarouselViewModel.swift
Outdated
Show resolved
Hide resolved
samwyndham
changed the title
Test Results1 992 tests 1 965 ✅ 2m 31s ⏱️ Results for commit 01bc21c. |
samwyndham
requested review from
a team,
netbe,
johnxnguyen and
jullianm
and removed request for
a team and
netbe
jullianm
approved these changes
Aug 8, 2025
| thumbnails[draft.versionID] = thumbnail | ||
| refreshItems() | ||
| } catch { | ||
| WireLogger.wireCells.error("Failed to generate thumbnail for file type: \(fileType.identifier)") |
There was a problem hiding this comment.
question: if we fail generating a thumbnail, what happens UI wise ?
There was a problem hiding this comment.
Good question.
The preview will keep showing an activity indicator. Now my assumption is that this is a very rare event which is why I haven't dealt with it specifically but I might be wrong.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
This PR adds thumbnails to the previews when uploading assets to wire cells
ScreenRecording_08-08-2025.08-44-19_1.mov
Testing
Run unit tests
Checklist
[WPB-XXX].