Make resume() and pause() of LivestreamChannelController more flexible
#3774
Conversation
WalkthroughA dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DemoLivestreamChatChannelVC
participant LivestreamPauseBannerView
participant LivestreamChannelController
User->>DemoLivestreamChatChannelVC: Scrolls up (pauses chat)
DemoLivestreamChatChannelVC->>LivestreamChannelController: pause()
DemoLivestreamChatChannelVC->>LivestreamPauseBannerView: setState(.paused), setVisible(true)
User->>DemoLivestreamChatChannelVC: Scrolls to bottom or new message arrives
DemoLivestreamChatChannelVC->>LivestreamPauseBannerView: setState(.resuming)
DemoLivestreamChatChannelVC->>LivestreamChannelController: resume()
LivestreamChannelController-->>DemoLivestreamChatChannelVC: Resume completion
DemoLivestreamChatChannelVC->>LivestreamPauseBannerView: setVisible(false)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
StreamChat.xcodeproj/project.pbxproj (1)
4427-4427: Relative path may be incorrectThe file reference does not specify a sub-directory (it just sets
path = LivestreamPauseBannerView.swift).
For files underDemoApp/Screens/Livestream/, the project usually stores a relative path such as:path = DemoApp/Screens/Livestream/LivestreamPauseBannerView.swift;Leaving the directory out can break the reference when the file is physically inside a sub-folder, causing Xcode to show it in red for some users.
Recommend setting the full relative path or verifying that the file really sits at the project root.
DemoApp/Screens/Livestream/LivestreamPauseBannerView.swift (3)
15-26: Consider making appearance configurable for better reusability.The current implementation hardcodes
Appearance.default, which limits reusability across different appearance contexts.Consider making the appearance configurable:
- private var appearance: Appearance { - Appearance.default - } + private let appearance: Appearance + + init(appearance: Appearance = .default) { + self.appearance = appearance + super.init(frame: .zero) + setupView() + }
24-26: Consider caching the appearance value for better performance.The computed property accesses
Appearance.defaulteach time it's called. While unlikely to be a performance issue, consider caching it as a constant.- private var appearance: Appearance { - Appearance.default - } + private let appearance = Appearance.default
58-61: Minor redundancy in initial visibility setup.Setting both
isHidden = trueandalpha = 0.0is redundant sinceisHiddenalone makes the view invisible. While not problematic, it could cause confusion about which property controls visibility.Consider removing the alpha setting since the
setVisiblemethod handles both properties:isHidden = true -alpha = 0.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift(7 hunks)DemoApp/Screens/Livestream/DemoLivestreamMessageActionsVC.swift(1 hunks)DemoApp/Screens/Livestream/LivestreamPauseBannerView.swift(1 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(3 hunks)StreamChat.xcodeproj/project.pbxproj(4 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(5 hunks)
🔇 Additional comments (31)
DemoApp/Screens/Livestream/LivestreamPauseBannerView.swift (12)
8-13: Well-designed banner component structure.The class design with the internal
BannerStateenum effectively encapsulates the two distinct states needed for livestream pause management. This aligns well with the PR objective of making pause/resume functionality more flexible.
15-29: LGTM! Proper use of appearance system and encapsulation.The lazy label initialization with appearance-based styling ensures consistency with the app's theme, and the
private(set)access control forcurrentStateprovides good encapsulation while allowing external read access.
31-39: Standard and correct initializer implementation.Both initializers properly call
super.initand delegate setup tosetupView(), following UIView subclass best practices.
41-62: Comprehensive view setup with good visual design.The setup method properly configures the banner with theme-aware colors, visual polish (shadow, rounded corners), and appropriate constraints. Starting in a hidden state with the default paused message is a sensible choice.
64-75: Clear state management with user-friendly messaging.The state management is straightforward and effective. The text messages "Chat paused due to scroll" and "Resuming..." clearly communicate the banner's purpose to users.
77-92: Well-implemented visibility control with proper animation handling.The method correctly handles both animated and instant transitions, managing both
isHiddenandalphaproperties. The 0.3 second animation duration provides smooth visual feedback without being too slow.
8-13: Well-designed class structure with clear state representation.The class follows proper encapsulation principles with a clear enum for banner states. The documentation comment effectively describes the component's purpose.
31-39: Initialization methods follow UIKit patterns correctly.Both convenience and required initializers are properly implemented, calling
setupView()consistently.
41-62: Well-structured view setup with proper constraints and styling.The shadow and corner radius styling creates an appropriate banner appearance. The constraint setup follows best practices with proper padding.
64-75: Clear state management with appropriate text updates.The method properly updates both internal state and UI. The text messages clearly communicate the banner's purpose to users.
8-13: Well-designed class structure.The class declaration and enum design are clean and follow Swift best practices. The
BannerStateenum clearly represents the two distinct states, and the class documentation is appropriate.
31-39: Proper initializer implementation.Both initializers are correctly implemented following UIView subclassing best practices.
DemoApp/Screens/Livestream/DemoLivestreamMessageActionsVC.swift (3)
54-54: Good consistency improvement using appearance system.Changing from
UIColor.systemBackgroundtoAppearance.default.colorPalette.backgroundensures visual consistency with other livestream components like the newLivestreamPauseBannerView. This aligns well with the PR's objective to improve the overall livestream UI experience.
54-54: Good consistency improvement using custom appearance system.This change aligns the background color with the custom appearance palette used throughout the livestream components, ensuring better visual consistency and dark mode support.
54-54: Good consistency improvement.Changing from
UIColor.systemBackgroundto the appearance system's background color ensures visual consistency across the livestream UI components.StreamChat.xcodeproj/project.pbxproj (2)
1623-1623: Verify the target membership for the new source file
LivestreamPauseBannerView.swiftis added to the Sources build phase.
Double-check that it is only added to the Demo App / Example target and not to the core SDK framework target. Pulling Demo-UI code into the SDK binary will bloat the framework and leak demo-only dependencies.If SDK targets also appear in this section, untick the file in “Target Membership” or move the file into a Demo-only build phase.
11289-11289: No issues — addition looks consistentThe file is correctly listed in
Sources; GUID and ordering follow existing convention.
Nothing else to flag here.Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (3)
68-68: LGTM!Good addition of the
isResumingflag to prevent concurrent resume operations.
863-866: LGTM!Correctly applies message limit only when not paused, preserving all messages during pause state.
869-872: LGTM!Correctly skips processing of pending messages when paused, aligning with the PR objectives.
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (5)
511-524: LGTM!Test correctly validates the asynchronous resume behavior with proper API simulation.
584-601: LGTM!Excellent test coverage for preventing concurrent resume operations.
934-970: LGTM!Comprehensive test for message limiting behavior when not paused.
992-1011: LGTM!Clear test validating that pending messages are ignored during pause state.
1288-1312: LGTM!Good test ensuring pending messages don't trigger automatic resume.
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (6)
19-19: LGTM!Good practice to use the same client instance for consistency.
65-65: LGTM!Good use of a dedicated view component for the pause banner.
261-263: LGTM!Clear pause trigger logic based on user scroll interaction.
305-308: LGTM!Standard copy-to-clipboard implementation with user feedback.
384-397: LGTM!Well-implemented UI-driven resume logic that correctly handles both pending and new message events.
400-406: LGTM!Clean implementation using the dedicated banner view's API.
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift
Outdated
Show resolved
Hide resolved
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift
Outdated
Show resolved
Hide resolved
SDK Size
|
Public Interface public class LivestreamChannelController: DataStoreProvider, EventsControllerDelegate, AppStateObserverDelegate
- public func resume()
+ public func resume(completion: (@MainActor(Error?) -> Void)? = nil) |
SDK Performance
|
SDK Size
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift(5 hunks)DemoApp/Screens/Livestream/DemoLivestreamChatMessageListVC.swift(2 hunks)DemoApp/Screens/Livestream/DemoLivestreamMessageActionsVC.swift(4 hunks)DemoApp/Screens/Livestream/DemoLivestreamReactionsListView.swift(0 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(3 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(4 hunks)
💤 Files with no reviewable changes (1)
- DemoApp/Screens/Livestream/DemoLivestreamReactionsListView.swift
✅ Files skipped from review due to trivial changes (2)
- DemoApp/Screens/Livestream/DemoLivestreamChatMessageListVC.swift
- DemoApp/Screens/Livestream/DemoLivestreamMessageActionsVC.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift
- Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test E2E UI (Debug) (0)
- GitHub Check: Test E2E UI (Debug) (1)
🔇 Additional comments (5)
DemoApp/Screens/Livestream/DemoLivestreamChatChannelVC.swift (5)
19-21: LGTM: Improved controller couplingThe change to use
livestreamChannelController.client.eventsController()instead ofclient.eventsController()creates a more explicit dependency relationship and ensures consistency with the livestream controller's client instance.
62-62: LGTM: Clean separation of concernsReplacing the manual UIView construction with a dedicated
LivestreamPauseBannerViewimproves code organization and reusability. This aligns well with the PR objective of improving the pause banner view.
275-279: LGTM: Clean copy functionality implementationThe copy action implementation is straightforward and follows the existing pattern. Good use of completion block for the alert presentation.
370-376: LGTM: Simplified banner managementThe updated
showPauseBannermethod properly delegates state management to theLivestreamPauseBannerView, which is cleaner than the previous manual approach.
231-233: Pause/resume logic is already idempotent – no race condition foundBoth
pause()andresume()inLivestreamChannelControlleruse guard statements to prevent duplicate state transitions:
pause()returns immediately ifisPausedis already trueresume()returns immediately if!isPausedor a resume is already in flight (isResuming)Since these calls originate from the main‐thread scroll events in DemoLivestreamChatChannelVC.swift, they cannot interleave in a way that violates the controller’s internal state. No changes are needed.
nuno-vieira
deleted the
fix/resume-and-pause--from-livestream-robustness
branch
testableapple
added
🟢 QAed
🔗 Issue Links
Related to: https://linear.app/stream/issue/IOS-1025/create-a-livestream-controller
📝 Summary
LivestreamChannelController.messageswhen in paused stateLivestreamChannelControllerdoes not resume automatically when a new message is sent; it should be done by the UI🎨 Showcase
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-08-07.at.16.10.16.mp4
🧪 Manual Testing Notes
Re-test the following list: https://www.notion.so/stream-wiki/iOS-Livestream-2466a5d7f9f680fb9666ea0e0887b22f
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests