Fix/gallery corner radius single image #3810
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughCorner-masking for gallery attachment previews moved from layout-time to content-update, now computed from layoutOptions and layout direction; single-image messages apply both left and right masked corners to the left container. A snapshot test validating single-image corner masking was added. Changes
Sequence Diagram(s)sequenceDiagram
actor ContentView as ChatMessageContentView
participant Injector as GalleryAttachmentViewInjector
participant Gallery as GalleryAttachmentGalleryView
participant Left as LeftPreviewsContainer
participant Right as RightPreviewsContainer
ContentView->>Injector: contentViewDidUpdateContent(layoutOptions)
alt layoutOptions present
Injector->>Gallery: read effectiveUserInterfaceLayoutDirection
Injector->>Injector: compute leftMaskedCorners & rightMaskedCorners
Injector->>Left: set cornerRadius=16, masksToBounds=true
Injector->>Right: set cornerRadius=16, masksToBounds=true
alt single attachment
Injector->>Left: apply left ∪ right maskedCorners
Injector->>Right: apply right maskedCorners
else multiple attachments
Injector->>Left: apply left maskedCorners
Injector->>Right: apply right maskedCorners
end
else no layoutOptions
note over Injector: do not modify maskedCorners
end
ContentView->>Injector: contentViewDidLayout()
note over Injector: aspect-ratio constraints unchanged (no corner work)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift (2)
610-647: Add a guard test: single gallery item plus non-gallery attachmentThis catches the bug where
allAttachments.count> 1 but gallery still renders a single item. Recommend adding:@@ func test_appearance_whenMessageHasOnlyOneImage_masksAllTopCorners() throws { class CustomBubbleView: ChatMessageBubbleView { override func updateContent() { super.updateContent() backgroundColor = .red } } @@ AssertSnapshot(view, variants: [.defaultLight]) } + + func test_appearance_whenMessageHasOneImage_andLinkPreview_masksAllTopCorners() throws { + class CustomBubbleView: ChatMessageBubbleView { + override func updateContent() { + super.updateContent() + backgroundColor = .red + } + } + var components = Components.default + components.messageBubbleView = CustomBubbleView.self + + let linkPayload = try JSONEncoder.stream.encode(LinkAttachmentPayload( + originalURL: URL(string: "https://getstream.io")!, + title: "Stream", + text: "Chat API", + author: "Stream", + previewURL: TestImages.r2.url + )) + + let message: ChatMessage = .mock( + id: .unique, + cid: .unique, + text: "Hello", + author: .unique, + createdAt: createdAt, + attachments: [ + .dummy( + id: .unique, + type: .image, + payload: try JSONEncoder.stream.encode(ImageAttachmentPayload(title: nil, imageRemoteURL: TestImages.r2.url)), + uploadingState: nil + ), + .dummy(id: .unique, type: .linkPreview, payload: linkPayload, uploadingState: nil) + ], + localState: nil, + isSentByCurrentUser: false + ) + + let view = contentView( + message: message, + channel: .mock(cid: .unique, membership: .mock(id: .unique)), + components: components, + attachmentInjector: GalleryAttachmentViewInjector.self + ) + + AssertSnapshot(view, variants: [.defaultLight]) + }
610-647: Optional: add RTL and outgoing variantsConsider asserting RTL and current-user cases to guard layout-direction differences.
Example:
- set
UIView.appearance().semanticContentAttribute = .forceRightToLeft- or pass a message with
isSentByCurrentUser: trueSources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift (2)
61-65: Avoid duplicate computation of rounded corners.Compute once, reuse for left/right intersections.
- let leftMaskedCorners = options.roundedCorners(for: galleryView.effectiveUserInterfaceLayoutDirection) - .intersection(leftCorners) + let rounded = options.roundedCorners(for: galleryView.effectiveUserInterfaceLayoutDirection) + let leftMaskedCorners = rounded.intersection(leftCorners) ... - let rightMaskedCorners = options.roundedCorners(for: galleryView.effectiveUserInterfaceLayoutDirection) - .intersection(rightCorners) + let rightMaskedCorners = rounded.intersection(rightCorners)
67-79: Derive corner radius (and curve) from the bubble view; avoid hard-coded 16.Aligns with theming and future changes to bubble rounding; also match
cornerCurvefor visual parity.- galleryView.leftPreviewsContainerView.layer.cornerRadius = 16 + let cornerRadius = contentView.bubbleView?.layer.cornerRadius ?? 16 + galleryView.leftPreviewsContainerView.layer.cornerRadius = cornerRadius + if #available(iOS 13.0, *) { + let curve = contentView.bubbleView?.layer.cornerCurve ?? .circular + galleryView.leftPreviewsContainerView.layer.cornerCurve = curve + galleryView.rightPreviewsContainerView.layer.cornerCurve = curve + } ... - galleryView.rightPreviewsContainerView.layer.cornerRadius = 16 + galleryView.rightPreviewsContainerView.layer.cornerRadius = cornerRadius
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/__Snapshots__/ChatMessageContentView_Tests/test_appearance_whenMessageHasOnlyOneImage_masksAllTopCorners.default-light.pngis excluded by!**/*.png
📒 Files selected for processing (2)
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift(1 hunks)Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Respect .swiftlint.yml and repo-specific SwiftLint rules when writing Swift code
Do not broadly suppress SwiftLint rules; scope and justify any exceptions
When altering public API, update inline documentation in the affected Swift files
Files:
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swiftSources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift
Tests/{StreamChatTests,StreamChatUITests}/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Add or extend tests in the matching module’s Tests folder when changing code
Files:
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift
Tests/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Prefer provided fakes/mocks from repo test helpers in tests
Files:
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift
Sources/StreamChatUI/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Ensure tests cover view controllers and UI behaviors in StreamChatUI
Files:
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift
🧠 Learnings (2)
📚 Learning: 2025-09-02T13:48:02.317Z
Learnt from: CR
PR: GetStream/stream-chat-swift#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T13:48:02.317Z
Learning: Applies to Sources/StreamChatUI/**/*.swift : Ensure tests cover view controllers and UI behaviors in StreamChatUI
Applied to files:
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift
📚 Learning: 2025-09-02T13:48:02.317Z
Learnt from: CR
PR: GetStream/stream-chat-swift#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T13:48:02.317Z
Learning: Applies to Sources/StreamChat/**/*.swift : Ensure tests cover core models and API surface of the StreamChat module
Applied to files:
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift
🧬 Code graph analysis (2)
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift (1)
Sources/StreamChatUI/ChatMessageList/ChatMessage/ChatMessageContentView.swift (1)
updateContent(593-747)
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift (2)
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift (1)
contentView(1266-1289)Sources/StreamChatUI/ChatMessageList/ChatMessage/ChatMessageContentView.swift (1)
roundedCorners(1094-1104)
🔇 Additional comments (2)
Tests/StreamChatUITests/SnapshotTests/ChatMessageList/ChatMessage/ChatMessageContentView_Tests.swift (1)
610-647: Good targeted snapshot; verifies the regression fixThe custom bubble background makes corner issues visible and the test is focused and readable.
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift (1)
57-80: Add RTL and flipped-layout snapshot coverage for masking.Ensure the new masking holds for
.flippedand RTL. Consider snapshots for:
- single gallery item, incoming/outgoing, RTL/LTR.
Do you want me to draft the snapshot tests mirroring
test_appearance_whenMessageHasOnlyOneImage_masksAllTopCornersfor RTL and.flipped?
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift
Show resolved
Hide resolved
Sources/StreamChatUI/ChatMessageList/Attachments/Gallery/GalleryAttachmentViewInjector.swift
Show resolved
Hide resolved
Generated by 🚫 Danger |
Public Interface🚀 No changes affecting the public interface. |
SDK Performance
|
SDK Size
|
testableapple
added
🟢 QAed
|
4 participants
🔗 Issue Links
https://linear.app/stream/issue/IOS-1119
🎯 Goal
Fix gallery view not masking the right top corner when there is only one image in the gallery.
🛠 Implementation
The problem is that the right masking was only applied to the right container. But when the gallery has only one image, it only contains the left container. So the masking needs to be applied to the left container.
Moved the masking to the update content function, since it now depends on content. Otherwise cell reuse could break the logic.
🧪 Manual Testing Notes
N/A. Covered by snapshot tests.
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
Bug Fixes
Tests