Redesign attachment uploading and error states#1408
Merged
nuno-vieira merged 18 commits intodevelopfrom Apr 10, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThis PR refactors the attachment upload and download UI by removing downloading state indicators, replacing bottom-right progress/error overlays with inline overlay UI rendering, introducing a new RetryBadgeView component for failed uploads, and enhancing LoadingSpinnerView to support determinate progress visualization. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Public Interface+ public struct RetryBadgeView: View
+
+ public var body: some View
+
+
+ public init()
public struct FileAttachmentDisplayView: View
- public init(url: URL,title: String,sizeString: String)
+ public init(url: URL,title: String,sizeString: String,uploadingState: AttachmentUploadingState? = nil,onRetry: (() -> Void)? = nil)
public struct MessageMediaAttachmentContentView: View
- public init(factory: Factory,source: MediaAttachment,width: CGFloat,height: CGFloat,cornerRadius: CGFloat? = nil,corners: UIRectCorner? = nil,isOutgoing: Bool = false)
+ public init(factory: Factory,source: MediaAttachment,width: CGFloat,height: CGFloat,cornerRadius: CGFloat? = nil,corners: UIRectCorner? = nil,isOutgoing: Bool = false,onUploadRetry: (() -> Void)? = nil)
public struct LoadingSpinnerView: View
- public init(size: CGFloat,bordered: Bool)
+ public init(size: CGFloat,bordered: Bool = false,progress: Double? = nil)
public enum LoadingSpinnerSize: Sendable
- @MainActor public static var small: CGFloat
+ @MainActor public static var medium: CGFloat
- @MainActor public static var extraSmall: CGFloat
+ @MainActor public static var small: CGFloat
+ @MainActor public static var extraSmall: CGFloat |
Collaborator
SDK Size
|
Collaborator
StreamChatSwiftUI XCSize
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentsContainerView.swift (1)
272-279: Consider adding error handling forresendMessage().The retry closure follows the same fire-and-forget pattern as
FileAttachmentView.retryUpload(), which is consistent. However,resendMessage()is an async operation that could fail. Consider whether users should receive feedback on retry failures.This is a minor suggestion since the pattern matches existing code in
FileAttachmentViewand the SDK likely handles state updates internally. If error handling is desired, it could be addressed in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentsContainerView.swift` around lines 272 - 279, The retry closure for onUploadRetry currently calls controller.resendMessage() fire-and-forget; change it to handle possible errors by calling the async resendMessage() method and capturing its result, e.g., call controller.resendMessage() within an async Task and handle failures by logging or surfacing a user-visible error; update the closure that builds the messageController (referencing message, chatClient, messageController, and resendMessage) to create Task { do { try await controller.resendMessage() } catch { /* handle: processLogger.error / show toast / update UI state */ } } so retry failures are not silently dropped.Sources/StreamChatSwiftUI/CommonViews/RetryBadgeView.swift (1)
20-38: Consider adding an accessibility label for VoiceOver users.The view has an
accessibilityIdentifierfor UI testing but lacks anaccessibilityLabelto describe the badge's purpose to VoiceOver users. Since this badge indicates a failed upload with retry capability, consider adding a descriptive label.♻️ Suggested improvement
.frame(width: size, height: size) .accessibilityIdentifier("RetryBadgeView") + .accessibilityLabel(L10n.Message.Sending.attachmentRetryUpload)As per coding guidelines: "Ensure components have accessibility labels, traits, and dynamic type support"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatSwiftUI/CommonViews/RetryBadgeView.swift` around lines 20 - 38, The RetryBadgeView's body currently sets only an accessibilityIdentifier but lacks an accessibilityLabel; update the view (in the RetryBadgeView body/ZStack) to add a descriptive accessibilityLabel like "Failed upload. Tap to retry" (localized) and appropriate accessibilityTraits (e.g., .isButton) so VoiceOver users understand the badge's purpose; ensure the label is localized and attach it to the same view that has accessibilityIdentifier("RetryBadgeView").Sources/StreamChatSwiftUI/ChatMessageList/FileAttachmentView.swift (2)
96-101: Consider adding error handling forresendMessage().The
resendMessage()call has no completion handler, so upload failures will occur silently without user feedback. The similar implementation inDefaultMessageActions.swift(lines 729-748) handles the completion callback to report errors.For consistency and better UX, consider adding error handling or logging:
Proposed improvement
private func retryUpload() { let messageId = attachment.id.messageId let cid = attachment.id.cid let controller = chatClient.messageController(cid: cid, messageId: messageId) - controller.resendMessage() + controller.resendMessage { error in + if let error { + log.error("Failed to resend message: \(error.localizedDescription)") + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatSwiftUI/ChatMessageList/FileAttachmentView.swift` around lines 96 - 101, The retryUpload() implementation calls controller.resendMessage() without handling its completion, so upload failures are silent; update retryUpload() to call resendMessage(completion:) on the MessageController returned by chatClient.messageController(cid:cid,messageId:) and handle the Result/Error in the completion (log the error via process/console or call the app’s error reporting, update attachment state or show user-facing feedback) similar to the error handling in DefaultMessageActions.swift (lines around DefaultMessageActions.resendMessage / resend flow) so failures are surfaced to the user and metrics.
193-200: Consider adding an accessibility label to the retry button.The retry button currently relies on the text content for accessibility. Adding an explicit accessibility label would improve VoiceOver support.
Proposed improvement
if let onRetry { Button(action: onRetry) { Text(L10n.Message.Sending.attachmentRetryUpload) .font(fonts.subheadline) .foregroundColor(Color(colors.textLink)) } .buttonStyle(.plain) + .accessibilityLabel(L10n.Message.Sending.attachmentRetryUpload) }Based on learnings: "Ensure components have accessibility labels, traits, and dynamic type support".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatSwiftUI/ChatMessageList/FileAttachmentView.swift` around lines 193 - 200, The retry Button inside FileAttachmentView (the optional onRetry Button) lacks an explicit accessibility label; update the Button by adding an accessibilityLabel that describes its action (e.g., "Retry attachment upload" or localized equivalent using L10n) and appropriate accessibilityTraits (e.g., .button) so VoiceOver announces it correctly; ensure you reference the optional onRetry closure and set the accessibility label on that Button instance to preserve dynamic type/localization.Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentContentView.swift (1)
147-155: Consider adding an accessibility label to the retry button.The
retryOverlayfunction wraps theRetryBadgeViewin aButton, but there's no accessibility label to describe the action to VoiceOver users.Proposed improvement
private func retryOverlay(action: `@escaping` () -> Void) -> some View { Button(action: action) { ZStack { Color(colors.backgroundCoreOverlayLight) RetryBadgeView() } } .buttonStyle(.plain) + .accessibilityLabel(L10n.Message.Sending.attachmentRetryUpload) }Based on learnings: "Ensure components have accessibility labels, traits, and dynamic type support".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentContentView.swift` around lines 147 - 155, The Button returned by retryOverlay lacks an accessibility label; update retryOverlay to add an accessibilityLabel (and optionally accessibilityHint and .accessibilityAddTraits(.isButton)) to the Button wrapping RetryBadgeView so VoiceOver users get a descriptive text like "Retry sending message" (use a localized string); reference the retryOverlay(action:) function and the RetryBadgeView so you add the accessibility modifiers directly on the Button before returning it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/StreamChatSwiftUI/ChatMessageList/FileAttachmentView.swift`:
- Around line 96-101: The retryUpload() implementation calls
controller.resendMessage() without handling its completion, so upload failures
are silent; update retryUpload() to call resendMessage(completion:) on the
MessageController returned by chatClient.messageController(cid:cid,messageId:)
and handle the Result/Error in the completion (log the error via process/console
or call the app’s error reporting, update attachment state or show user-facing
feedback) similar to the error handling in DefaultMessageActions.swift (lines
around DefaultMessageActions.resendMessage / resend flow) so failures are
surfaced to the user and metrics.
- Around line 193-200: The retry Button inside FileAttachmentView (the optional
onRetry Button) lacks an explicit accessibility label; update the Button by
adding an accessibilityLabel that describes its action (e.g., "Retry attachment
upload" or localized equivalent using L10n) and appropriate accessibilityTraits
(e.g., .button) so VoiceOver announces it correctly; ensure you reference the
optional onRetry closure and set the accessibility label on that Button instance
to preserve dynamic type/localization.
In
`@Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentContentView.swift`:
- Around line 147-155: The Button returned by retryOverlay lacks an
accessibility label; update retryOverlay to add an accessibilityLabel (and
optionally accessibilityHint and .accessibilityAddTraits(.isButton)) to the
Button wrapping RetryBadgeView so VoiceOver users get a descriptive text like
"Retry sending message" (use a localized string); reference the
retryOverlay(action:) function and the RetryBadgeView so you add the
accessibility modifiers directly on the Button before returning it.
In
`@Sources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentsContainerView.swift`:
- Around line 272-279: The retry closure for onUploadRetry currently calls
controller.resendMessage() fire-and-forget; change it to handle possible errors
by calling the async resendMessage() method and capturing its result, e.g., call
controller.resendMessage() within an async Task and handle failures by logging
or surfacing a user-visible error; update the closure that builds the
messageController (referencing message, chatClient, messageController, and
resendMessage) to create Task { do { try await controller.resendMessage() }
catch { /* handle: processLogger.error / show toast / update UI state */ } } so
retry failures are not silently dropped.
In `@Sources/StreamChatSwiftUI/CommonViews/RetryBadgeView.swift`:
- Around line 20-38: The RetryBadgeView's body currently sets only an
accessibilityIdentifier but lacks an accessibilityLabel; update the view (in the
RetryBadgeView body/ZStack) to add a descriptive accessibilityLabel like "Failed
upload. Tap to retry" (localized) and appropriate accessibilityTraits (e.g.,
.isButton) so VoiceOver users understand the badge's purpose; ensure the label
is localized and attach it to the same view that has
accessibilityIdentifier("RetryBadgeView").
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1108b05-a40f-408a-a823-d9f9e4535a70
⛔ Files ignored due to path filters (20)
Sources/StreamChatSwiftUI/Generated/L10n.swiftis excluded by!**/generated/**StreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_downloadDisabled.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_downloadedState.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_downloadingState.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploaded_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploaded_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploadingFailed_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploadingFailed_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploadingProgress_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/FileAttachmentView_Tests/test_fileAttachmentView_uploadingProgress_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_failedWhenMessageTextIsEmpty_snapshot.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_failed_snapshot.1.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_uploadingWithoutText_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_uploadingWithoutText_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_uploading_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_imageAttachments_uploading_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_multipleImageAttachments_failed_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_multipleImageAttachments_failed_snapshot.default-light.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_multipleImageAttachments_uploading_snapshot.default-dark.pngis excluded by!**/*.pngStreamChatSwiftUITests/Tests/ChatChannel/__Snapshots__/MessageItemView_Tests/test_multipleImageAttachments_uploading_snapshot.default-light.pngis excluded by!**/*.png
📒 Files selected for processing (14)
CHANGELOG.mdSources/StreamChatSwiftUI/ChatChannel/ChannelInfo/FileAttachmentsView.swiftSources/StreamChatSwiftUI/ChatMessageList/AttachmentDownloadingStateView.swiftSources/StreamChatSwiftUI/ChatMessageList/AttachmentUploadingStateView.swiftSources/StreamChatSwiftUI/ChatMessageList/FileAttachmentView.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentContentView.swiftSources/StreamChatSwiftUI/ChatMessageList/MessageMediaAttachmentsContainerView.swiftSources/StreamChatSwiftUI/ChatMessageList/PercentageProgressView.swiftSources/StreamChatSwiftUI/CommonViews/LoadingSpinnerView.swiftSources/StreamChatSwiftUI/CommonViews/RetryBadgeView.swiftSources/StreamChatSwiftUI/Resources/en.lproj/Localizable.stringsStreamChatSwiftUITests/Tests/ChatChannel/ChatChannelTestHelpers.swiftStreamChatSwiftUITests/Tests/ChatChannel/FileAttachmentView_Tests.swiftStreamChatSwiftUITests/Tests/ChatChannel/MessageItemView_Tests.swift
💤 Files with no reviewable changes (2)
- Sources/StreamChatSwiftUI/ChatMessageList/PercentageProgressView.swift
- Sources/StreamChatSwiftUI/ChatMessageList/AttachmentDownloadingStateView.swift
martinmitrevski
approved these changes
Apr 10, 2026
Contributor
martinmitrevski
left a comment
martinmitrevski
left a comment
There was a problem hiding this comment.
LGTM ✅ I would just move the resend logic to the vm
| private func retryUpload() { | ||
| let messageId = attachment.id.messageId | ||
| let cid = attachment.id.cid | ||
| let controller = chatClient.messageController(cid: cid, messageId: messageId) |
Contributor
There was a problem hiding this comment.
hmm, is this ideal here? Shall we move it to the VM via the options?
Member
Author
There was a problem hiding this comment.
We don't have any view model here. I can set it up, but I'm not sure what additional changes will be required
Contributor
There was a problem hiding this comment.
no, that's not what I meant. Just put it a handler onRetry or similar in the FileAttachmentViewOptions, and the factory method will pass the view model resend method. So just the method, not the whole VM - that will slow things down a lot.
| isOutgoing: message.isSentByCurrentUser, | ||
| onUploadRetry: item.uploadingState?.state == .uploadingFailed ? { [message, chatClient] in | ||
| guard let cid = message.cid else { return } | ||
| let controller = chatClient.messageController( |
Contributor
There was a problem hiding this comment.
similarly here, we can have one method in the vm doing this
Member
Author
There was a problem hiding this comment.
Same here, there is no view model on this view, so we need to pass an existing one or create one for this.
I think we can do it later if we think it is worth it and customers request it
Contributor
There was a problem hiding this comment.
anyway, if it's a lot of changes, ignore it
Member
Author
There was a problem hiding this comment.
I've tried to change this, but TBH I prefer the current solution for now. Without a proper view model, it will just forward the action to the factory, which is not good. The factory should just create things, not do actions
|
nuno-vieira
deleted the
add/attachments-error-and-uploading-state-redesign
branch
6 tasks
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 Links
IOS-1610
🎯 Goal
Redesign the uploading progress and error state indicators for media and file attachments to match the updated Figma design system specs.
📝 Summary
LoadingSpinnerViewthat supports both indeterminate (loading) and determinate (upload progress) modesRetryBadgeViewfor upload and thumbnail load failures on media attachments, with tap-to-retry supportFileAttachmentDisplayViewPercentageProgressViewandAttachmentDownloadingStateView(no longer used)🛠 Implementation
LoadingSpinnerView now accepts an optional
progress: Double?. Whennil, it renders the existing indeterminate spinning animation. When set, it draws a determinate arc proportional to the value (0–1). Predefined sizes include a new.medium(24pt) tier.RetryBadgeView is a new 32×32 red circular badge with a white border and a retry icon, shown when an upload or thumbnail load fails. Tapping it on media attachments retries the thumbnail load; tapping it on file attachments triggers
resendMessage().MessageMediaAttachmentContentView was refactored to manage overlay states internally — the
withUploadingStateIndicatormodifier is no longer applied at the container level for media. The video play icon is now hidden during uploads and errors. A loading-error overlay stops the shimmer and shows the retry badge.FileAttachmentDisplayView was extended with optional
uploadingStateandonRetryparameters so the same view handles the normal, uploading, and failed layouts without duplicating the file row structure.FileAttachmentViewdelegates to it, passing upload state and a retry closure.🧪 Manual Testing Notes
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Improvements
Localization