-
Notifications
You must be signed in to change notification settings - Fork 46
feat(ui): added support for multiple hosts and screen sharing to LivestreamPlayer #1125
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.
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
Conversation
WalkthroughAdds a shared JSON parser for capabilities_by_role and implements multi-host livestream UI with configurable layouts, screen-share modes, pluggable reconnect overlays/builders, theme spacing fields, and related API surface changes across Player/Content/Call UI. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1125 +/- ##
========================================
- Coverage 6.32% 6.31% -0.02%
========================================
Files 594 595 +1
Lines 40953 41085 +132
========================================
+ Hits 2592 2595 +3
- Misses 38361 38490 +129 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_video_flutter/lib/src/theme/stream_livestream_theme.dart (1)
304-324:hashCodeis missing the new grid-related fields.The
hashCodeimplementation usesObject.hashbut does not includehostsGridPadding,hostsGridMainAxisSpacing, andhostsGridCrossAxisSpacing. This will cause incorrect equality behavior when these fields differ.Apply this diff to include the new fields:
@override int get hashCode => Object.hash( playIconTheme, pauseIconTheme, playPauseIconSize, liveButtonColor, backstageButtonColor, callStateButtonTextStyle, participantCountTextStyle, durationTextStyle, backstageTextStyle, backstageCounterTextStyle, backstageParticipantsTextStyle, liveEndedTextStyle, liveEndedRecordingsTextStyle, participantIconTheme, speakerEnabledIconTheme, speakerDisabledIconTheme, expandIconTheme, contractIconTheme, + hostsGridPadding, + hostsGridMainAxisSpacing, + hostsGridCrossAxisSpacing, );Note:
Object.hashsupports up to 20 parameters. With 21 fields, consider usingObject.hashAll([...])instead.
🧹 Nitpick comments (4)
packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart (1)
23-33: Consider making the constructorconstfor immutability.Since all fields are
final, addingconstto the constructor enables compile-time constant creation and aligns with Flutter's immutable data patterns.class CallNotConnectedProperties { - CallNotConnectedProperties( + const CallNotConnectedProperties( this.call, { required this.isMigrating, required this.isReconnecting, });packages/stream_video/lib/open_api/video/coordinator/model/call_member_updated_permission_event.dart (1)
101-101: Good refactoring to use the helper method.The extraction of capabilities parsing logic improves maintainability. However, for consistency with the rest of
fromJson, consider using raw string notation.Apply this diff for consistency:
- : capabilitiesFromJson(json['capabilities_by_role']), + : capabilitiesFromJson(json[r'capabilities_by_role']),packages/stream_video/lib/open_api/video/coordinator/model/video_event.dart (1)
487-487: Good refactoring to use the helper method.The extraction of capabilities parsing logic improves maintainability. However, for consistency with the rest of
fromJson, consider using raw string notation.Apply this diff for consistency:
- : capabilitiesFromJson(json['capabilities_by_role']), + : capabilitiesFromJson(json[r'capabilities_by_role']),packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (1)
148-159: Consider comparing byuniqueParticipantKeyinstead ofuserId.The comparison
_screenShareHost?.userId == host.userIdmay incorrectly match if a user has multiple sessions. For consistency with the rest of the code that usesuniqueParticipantKey, consider:Widget _hostVideoContentBuilder( BuildContext context, Call call, CallParticipantState host, ) { if (widget.screenShareMode == LivestreamScreenShareMode.grid && - _screenShareHost?.userId == host.userId) { + _screenShareHost?.uniqueParticipantKey == host.uniqueParticipantKey) { return _buildScreenShareContent(_screenShareHost!); } return widget.callParticipantBuilder(context, call, host); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
packages/stream_video/lib/open_api/video/coordinator/model/call_member_updated_permission_event.dart(2 hunks)packages/stream_video/lib/open_api/video/coordinator/model/video_event.dart(2 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart(1 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart(5 hunks)packages/stream_video_flutter/lib/src/livestream/livestream_content.dart(8 hunks)packages/stream_video_flutter/lib/src/livestream/livestream_player.dart(4 hunks)packages/stream_video_flutter/lib/src/theme/stream_livestream_theme.dart(8 hunks)packages/stream_video_flutter/lib/stream_video_flutter.dart(1 hunks)
⏰ 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). (3)
- GitHub Check: build
- GitHub Check: stream_video_flutter
- GitHub Check: iOS Build (Dogfooding)
🔇 Additional comments (15)
packages/stream_video_flutter/CHANGELOG.md (1)
1-14: LGTM! Changelog entries are clear, comprehensive, and well-structured.The Unreleased section accurately documents all new features from the PR: multi-host and screen sharing support, new flags, builder APIs, videoFit parameter, and reconnect UI customization for both livestream and call screens. The descriptions are user-facing and follow the existing changelog conventions throughout the file.
packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart (4)
11-21: LGTM!The builder typedefs follow standard Flutter conventions and provide clear, well-named APIs for customization.
48-49: LGTM!The optional builder fields are well-documented and properly integrated into the widget's public API, maintaining backward compatibility.
Also applies to: 73-77
197-211: Note:isMigratingwill always befalsein this context.The
elsebranch at line 195 only executes when the status is not connected, not fast-reconnecting, and not migrating. Therefore,_status.isMigratingat line 202 will always befalse.If this is intentional for API completeness/future-proofing, consider adding a brief comment. Otherwise, verify whether the condition at line 167-169 should be adjusted to allow migrating states to reach this builder.
236-249: LGTM!The fast-reconnecting overlay integration follows the established builder pattern with a sensible default fallback, preserving backward compatibility.
packages/stream_video_flutter/lib/src/theme/stream_livestream_theme.dart (1)
69-72: LGTM!The new theme properties for grid layout (
hostsGridPadding,hostsGridMainAxisSpacing,hostsGridCrossAxisSpacing) are well-documented with sensible defaults of 4.0. The implementation acrosscopyWith,lerp,==,debugFillProperties, andmergeis consistent and correct.Also applies to: 128-136
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (3)
10-16: LGTM!The
LivestreamScreenShareModeenum is well-documented, providing clear distinction betweenspotlight(screen share as overlay) andgrid(screen share replaces host video in grid).
95-137: LGTM!The participant recalculation logic is well-structured:
- Filters participants with video or screen share enabled
- Maintains stable ordering by tracking previous participant keys
- Uses
mergeSortfor stable sorting after applying the previous order- Correctly guards
setStatewithmountedcheck
161-184: LGTM!The build method correctly handles the different layout scenarios:
- Spotlight screen share mode renders full-screen share content
- PiP/spotlight layouts render a single host
- Grid mode uses
CallParticipantsGridViewwith theme-provided spacingpackages/stream_video_flutter/lib/stream_video_flutter.dart (1)
31-31: LGTM!The new export for
livestream_hosts.dartis correctly placed in alphabetical order within the call_participants exports, makingStreamLivestreamHostsandLivestreamScreenShareModepart of the public API.packages/stream_video_flutter/lib/src/livestream/livestream_player.dart (2)
87-98: LGTM!New parameters are well-integrated with sensible defaults that maintain backward compatibility:
showMultipleHosts = falsepreserves single-host behaviorlayoutMode = ParticipantLayoutMode.gridandscreenShareMode = LivestreamScreenShareMode.spotlightprovide intuitive defaults
437-447: LGTM!New parameters are correctly propagated to
LivestreamContent. ThevideoFitlogic appropriately overrides toVideoFit.coverwhen in fullscreen mode while respectingwidget.videoFitotherwise.packages/stream_video_flutter/lib/src/livestream/livestream_content.dart (3)
26-42: LGTM!The new typedefs follow the established patterns in the codebase and provide clear, flexible customization points for:
- Fast reconnecting overlay display
- Host video rendering
- Host participant filtering
207-224: LGTM!The
_defaultHostsParticipantBuildercorrectly configuresStreamLivestreamHostsbased onshowMultipleHosts:
- Uses spotlight mode for single host, respects
layoutModefor multiple hosts- Passes only the first host when
showMultipleHostsis false- Includes a sensible default
screenShareContentBuilder
283-309: LGTM!The updated rendering logic correctly:
- Applies the custom or default host participant builder
- Shows a fast reconnecting overlay when appropriate
- Provides a sensible default overlay (small spinner at top-left)
.../stream_video/lib/open_api/video/coordinator/model/call_member_updated_permission_event.dart
Show resolved
Hide resolved
packages/stream_video/lib/open_api/video/coordinator/model/video_event.dart
Show resolved
Hide resolved
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart
Outdated
Show resolved
Hide resolved
packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart
Show resolved
Hide resolved
packages/stream_video_flutter/lib/src/livestream/livestream_content.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/stream_video_flutter/lib/src/livestream/livestream_content.dart (1)
26-43: Consider using properties classes for builder typedefs for future extensibility.The new
LivestreamFastReconnectingOverlayBuilderandLivestreamHostsParticipantBuildertypedefs pass parameters directly rather than using a properties object pattern likeLivestreamHostsUnavailablePropertiesandLivestreamNotConnectedProperties. This makes it harder to extend the API without breaking changes.-typedef LivestreamFastReconnectingOverlayBuilder = - Widget Function( - BuildContext context, - Call call, - ); +class LivestreamFastReconnectingProperties { + LivestreamFastReconnectingProperties(this.call); + final Call call; +} + +typedef LivestreamFastReconnectingOverlayBuilder = + Widget Function( + BuildContext context, + LivestreamFastReconnectingProperties properties, + );packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart (1)
17-22: Consider using a properties class forCallFastReconnectingOverlayBuilderfor consistency and future extensibility.
CallNotConnectedBuilderusesCallNotConnectedProperties, butCallFastReconnectingOverlayBuilderpassesCalldirectly. Using a properties class would make it easier to add parameters later without breaking the API.+class CallFastReconnectingProperties { + CallFastReconnectingProperties(this.call); + final Call call; +} + typedef CallFastReconnectingOverlayBuilder = Widget Function( BuildContext context, - Call call, + CallFastReconnectingProperties properties, );
🧹 Nitpick comments (1)
packages/stream_video_flutter/CHANGELOG.md (1)
4-4: Minor: Consider hyphenating "screen-sharing" for consistency.The changelog uses "screen sharing" (line 4) but "screen-sharing" appears hyphenated elsewhere in the file (e.g., line 101). Consider using "screen-sharing" consistently.
-* Livestream: Added multiple-host and screen sharing support in `LivestreamPlayer` and `LivestreamContent`. +* Livestream: Added multiple-host and screen-sharing support in `LivestreamPlayer` and `LivestreamContent`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart(5 hunks)packages/stream_video_flutter/lib/src/livestream/livestream_content.dart(8 hunks)packages/stream_video_flutter/lib/stream_video_flutter.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_flutter/lib/stream_video_flutter.dart
🧰 Additional context used
🪛 LanguageTool
packages/stream_video_flutter/CHANGELOG.md
[grammar] ~4-~4: Use a hyphen to join words.
Context: ...vestream: Added multiple-host and screen sharing support in LivestreamPlayer an...
(QB_NEW_EN_HYPHEN)
⏰ 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). (6)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: build
🔇 Additional comments (7)
packages/stream_video_flutter/lib/src/livestream/livestream_content.dart (3)
207-224: LGTM!The default hosts participant builder is well-structured. The
hosts.firstaccess at line 214 is safe because this builder is only invoked whenstreamingParticipantsis non-empty (guarded by the check at line 250).
99-107: LGTM!The new constructor parameters are well-designed with sensible defaults that maintain backward compatibility.
280-306: LGTM!The integration of custom builders with fallbacks to default implementations is well-structured and maintains backward compatibility.
packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart (3)
23-33: LGTM!The
CallNotConnectedPropertiesclass follows the established pattern and provides useful connection state information.
196-210: LGTM!The
callNotConnectedBuilderintegration properly constructsCallNotConnectedPropertieswith connection state flags, and falls back to the original behavior when not provided.
236-248: LGTM!The fast reconnecting overlay integration follows the established pattern with proper fallback to the default CircularProgressIndicator.
packages/stream_video_flutter/CHANGELOG.md (1)
1-13: LGTM - changelog entries accurately document the new features.The unreleased section properly documents the multi-host support, screen sharing, and reconnect UI customization features added in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/stream_video_flutter/lib/src/livestream/livestream_content.dart (1)
54-57: Consider wrapping filter parameters in a properties object for consistency.The
LivestreamHostsParticipantsFiltertakes a raw list parameter, while other builders in this file use properties objects (e.g.,LivestreamFastReconnectingProperties,LivestreamHostsParticipantProperties). For API consistency and future extensibility, consider introducing a properties class like:typedef LivestreamHostsParticipantsFilter = List<CallParticipantState> Function( LivestreamHostsParticipantsFilterProperties properties, ); class LivestreamHostsParticipantsFilterProperties { LivestreamHostsParticipantsFilterProperties({ required this.call, required this.allCallParticipants, }); final Call call; final List<CallParticipantState> allCallParticipants; }This would align with the pattern used by other builders and provide room to add context (like
call) if needed later.Based on learnings from past reviews, this consistency concern was previously raised.
🧹 Nitpick comments (5)
packages/stream_video_flutter/CHANGELOG.md (2)
5-5: Hyphenate "screen-sharing" for consistency with "multiple-host".For parallel structure and consistency across the CHANGELOG, "screen sharing" should be hyphenated to "screen-sharing" when used in compound contexts like "multiple-host and screen-sharing support."
Apply this change:
-* Added multiple-host and screen sharing support in `LivestreamPlayer` and `LivestreamContent`. +* Added multiple-host and screen-sharing support in `LivestreamPlayer` and `LivestreamContent`.
11-11: Clarify how the flags are exposed to the builder.Line 11 could be more precise about how
isMigrating/isReconnectingare provided. These flags are part of a properties object passed to the builder, not directly provided by the builder itself.Apply this change for clarity:
-* Added `videoFit` parameter in `LivestreamPlayer` to control contain/cover behavior when not in fullscreen. -* Livestream reconnect UI customization: - - `LivestreamPlayer`/`LivestreamContent`: added `livestreamFastReconnectingOverlayBuilder` to customize the UI shown during fast reconnect. - - `LivestreamContent`: `livestreamNotConnectedBuilder` provides `isMigrating`/`isReconnecting` flags to tailor messaging when not connected. +* Added `videoFit` parameter in `LivestreamPlayer` to control contain/cover behavior when not in fullscreen. +* Livestream reconnect UI customization: + - `LivestreamPlayer`/`LivestreamContent`: added `livestreamFastReconnectingOverlayBuilder` to customize the UI shown during fast reconnect. + - `LivestreamContent`: `livestreamNotConnectedBuilder` receives properties with `isMigrating`/`isReconnecting` flags to tailor messaging when not connected.packages/stream_video/test/src/location/location_service_test.dart (2)
89-89: Consider explicitly specifyingmaxAttempts: 3for test clarity.This test verifies exactly 3 calls (line 93) but now relies implicitly on the default
maxAttemptsvalue. If the default changes in the implementation, this test will fail in a non-obvious way. Explicitly specifyingmaxAttempts: 3would make the test's intent clearer and decouple it from implementation defaults.Apply this diff to restore explicit parameter:
- final result = await service.getLocation(); + final result = await service.getLocation(maxAttempts: 3);
172-172: Consider explicitly specifyingmaxAttempts: 3to match the test's design.The mock is specifically designed to fail twice and succeed on the 3rd attempt (line 159:
if (callCount < 3)), making this test inherently tied to 3 attempts. Removing the explicitmaxAttempts: 3parameter obscures this relationship and creates an implicit dependency on the default value. The test would be clearer and more maintainable with an explicit parameter.Apply this diff to restore explicit parameter:
- final result = await service.getLocation(); + final result = await service.getLocation(maxAttempts: 3);packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (1)
86-95: Consider removing redundanttoList()calls.Both
widget.hostsandoldWidget.hostsare already of typeList<CallParticipantState>, so the.toList()conversions on lines 90-91 are unnecessary and create extra list copies.Apply this diff:
if (!const ListEquality<CallParticipantState>().equals( - widget.hosts.toList(), - oldWidget.hosts.toList(), + widget.hosts, + oldWidget.hosts, )) { recalculateParticipants(widget.hosts); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/stream_video/test/src/location/location_service_test.dart(2 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart(1 hunks)packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart(5 hunks)packages/stream_video_flutter/lib/src/livestream/livestream_content.dart(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_flutter/lib/src/call_screen/call_content/call_content.dart
🧰 Additional context used
🪛 LanguageTool
packages/stream_video_flutter/CHANGELOG.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...sabled. * Added multiple-host and screen sharing support in LivestreamPlayer an...
(QB_NEW_EN_HYPHEN)
⏰ 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). (7)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: analyze_legacy_version
🔇 Additional comments (11)
packages/stream_video_flutter/CHANGELOG.md (1)
3-14: Comprehensive feature documentation for multi-host livestream support.The CHANGELOG entries clearly document the new features, parameters, and builders added in this release. The descriptions are detailed and align well with the PR objectives for multi-host and screen-sharing enhancements.
packages/stream_video/test/src/location/location_service_test.dart (1)
89-89: DefaultmaxAttemptsvalue is 3. Both modified tests at lines 89 and 172 rely on this default, which is correctly set inLocationService.getLocation().packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (3)
10-16: LGTM!The enum is well-documented with clear semantics for both screen-share modes.
21-52: LGTM!The constructor parameters are well-structured with sensible defaults. The conditional default for
sortbased onlayoutMode.sortingprovides good flexibility.
119-146: LGTM!The build method correctly prioritizes rendering: spotlight screen-share first, then single-participant spotlight/PiP layouts, and finally grid view. The integration with theme properties for spacing is appropriate.
packages/stream_video_flutter/lib/src/livestream/livestream_content.dart (6)
20-34: LGTM!The property classes follow a clear pattern for passing contextual data to builder functions.
114-122: LGTM!The new constructor parameters provide good extensibility while maintaining backward compatibility through sensible defaults.
151-188: LGTM!The field declarations are well-documented, explaining the relationship between
showMultipleHosts,layoutMode, and the default behavior.
202-241: LGTM!The default builders are well-structured. The
_defaultHostsParticipantBuildercorrectly adapts toshowMultipleHosts, ensuring backward compatibility by forcing spotlight mode and single-host display whenshowMultipleHostsis false.
263-265: LGTM!The filtering logic correctly uses the custom filter if provided, falling back to the default implementation.
280-327: LGTM!The body widget correctly layers the UI elements: platform-specific PiP views at the bottom, host participants in the middle, and the fast-reconnecting overlay on top when needed. The builder pattern with sensible fallbacks is well-implemented.
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (1)
86-95: Remove redundant.toList()calls.Since
hostsis already declared asList<CallParticipantState>(line 36), the.toList()calls at lines 89-91 are redundant and create unnecessary list copies.Apply this diff:
if (!const ListEquality<CallParticipantState>().equals( - widget.hosts.toList(), - oldWidget.hosts.toList(), + widget.hosts, + oldWidget.hosts, )) { recalculateParticipants(widget.hosts); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart(1 hunks)
⏰ 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). (7)
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_push_notification
🔇 Additional comments (4)
packages/stream_video_flutter/lib/src/call_participants/livestream_hosts.dart (4)
10-16: LGTM: Clear enum definition.The
LivestreamScreenShareModeenum is well-documented and provides two distinct modes for handling screen sharing in livestreams.
21-51: LGTM: Well-designed API surface.The constructor provides sensible defaults and clear property documentation. The fallback from custom sort to
layoutMode.sortingat line 30 is a good design choice.
106-118: Past concern addressed: usinguniqueParticipantKeyfor participant matching.The code now uses
uniqueParticipantKeyfor comparing participants (lines 112-113), which properly encapsulates bothuserIdandsessionId. This addresses the past review comment that flagged potential issues with single-userId comparisons when a user has multiple sessions.
121-147: LGTM: Build method implements clear rendering logic.The build method correctly handles the different combinations of screen share modes and layout modes:
- Spotlight screen share takes precedence (lines 124-127)
- Spotlight/PiP layouts show the first sorted participant (lines 129-137)
- Grid layout delegates to
_hostVideoContentBuilderwhich handles grid-mode screen sharing (lines 139-146)The null-safety usage is correct, and the theme property references align with the new multi-host feature.
Summary by CodeRabbit
New Features
Style / Theme
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.