Conversation
…ng settings - Fix preview disappearing in source picker (stale object reference) - Add double-click fullscreen on stream preview - Redesign hidden stream thumbnail with pulsing LIVE badge and hover effects - Enforce bitrate on RTP sender level + live encoding updates - Add mic noise suppression setting (AudioCaptureOptions) - Make audio ducking toggleable in settings (default: disabled) - New multi-stream layout with primary + scrollable mini-previews - Add 15 new unit tests for settings and encoding options - All 251 tests passing
- Move AudioDuckingService.disableWindowsDucking() to before room.connect() so Windows never gets a chance to trigger ducking on the new audio session - Also disable ducking again after RoomReconnectedEvent (Windows re-applies it) - Fix integration test 15: increase scroll iterations from 10 to 25, use smaller 150px steps, and break only when both 'Current Connection' and 'WebSocket URL' are simultaneously visible (ensuring all 4 labels in view) - All 25 integration tests and 251 unit tests pass
There was a problem hiding this comment.
Pull request overview
This PR extends the screen sharing feature with improved multi-stream viewing UX, adds audio-related user settings (noise suppression + Windows audio ducking), and strengthens test coverage around these behaviors.
Changes:
- Add multi-stream screen share layout (primary stream + secondary preview strip) and enhance screen share thumbnails/interaction.
- Add persisted settings for WebRTC noise suppression and (Windows) audio ducking disable, wire them into LiveKit connection options + settings UI, and localize new strings.
- Add/adjust unit + integration tests for the new screen share publish options and settings persistence.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/test/features/voice/models/screen_share_models_test.dart | Adds tests for ScreenShareSettings.toPublishOptions() encoding defaults and presets. |
| app/test/features/settings/settings_provider_test.dart | Adds tests verifying persistence/defaults for noise suppression and audio ducking settings. |
| app/lib/features/voice/widgets/voice_channel_view.dart | Replaces multi-share grid with a new primary/secondary multi-stream layout. |
| app/lib/features/voice/widgets/screen_source_picker_dialog.dart | Keeps selected source synced to refreshed capturer objects so thumbnails stay current. |
| app/lib/features/voice/widgets/screen_share_view.dart | Adds double-tap fullscreen and redesigns ScreenShareThumbnail with hover + pulsing LIVE badge. |
| app/lib/features/voice/providers/screen_share_provider.dart | Applies encoding changes to an active share and enforces bitrate/fps via RTP sender params. |
| app/lib/features/settings/screens/settings_screen.dart | Adds toggles for noise suppression and audio ducking disable. |
| app/lib/core/services/voice_service.dart | Applies noise suppression capture options and disables Windows ducking on connect/reconnect. |
| app/lib/core/providers/settings_provider.dart | Adds persisted fields + update/reset methods for new audio settings. |
| app/lib/core/l10n/strings_en.dart | Adds EN strings for new settings labels/descriptions. |
| app/lib/core/l10n/strings_de.dart | Adds DE strings for new settings labels/descriptions. |
| app/lib/core/l10n/app_localizations.dart | Exposes the new localization getters. |
| app/integration_test/app_test.dart | Makes settings scroll logic more robust in the integration test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| import 'package:concord_app/core/providers/settings_provider.dart'; | ||
| import 'package:concord_app/core/providers/websocket_provider.dart'; | ||
| import 'package:concord_app/features/auth/providers/auth_provider.dart'; | ||
| import 'package:concord_app/core/services/audio_ducking_service.dart'; |
There was a problem hiding this comment.
AudioDuckingService is imported unconditionally, but its implementation pulls in dart:io, dart:ffi, and win32, which will fail to compile for Flutter Web. Please refactor to a conditional import/export setup (e.g., a no-op stub for web/non-io) so voice_service.dart can be built on all supported platforms.
| import 'package:concord_app/core/services/audio_ducking_service.dart'; | |
| import 'package:concord_app/core/services/audio_ducking_service.dart' | |
| if (dart.library.html) | |
| 'package:concord_app/core/services/audio_ducking_service_stub.dart'; |
| // Audio ducking toggle (Windows only) | ||
| SwitchListTile( | ||
| title: Text(l10n.audioDuckingLabel), | ||
| subtitle: Text( | ||
| l10n.audioDuckingDescription, | ||
| style: const TextStyle(color: Colors.grey, fontSize: 12), | ||
| ), | ||
| value: settings.audioDuckingDisabled, | ||
| activeColor: const Color(0xFF5865F2), | ||
| contentPadding: EdgeInsets.zero, | ||
| onChanged: (value) { | ||
| ref.read(settingsProvider.notifier).updateAudioDuckingDisabled(value); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
The “Disable Audio Ducking” toggle is labeled as Windows-specific, but it’s currently shown (and writable) on all platforms. Consider gating it behind a Windows platform check (or disabling the control with a tooltip) to avoid confusing users on macOS/Linux/web where the setting has no effect.
| )..repeat(reverse: true); | ||
| _pulseAnim = Tween<double>(begin: 0.6, end: 1.0).animate( | ||
| CurvedAnimation(parent: _pulseController, curve: Curves.easeInOut), | ||
| ); | ||
| } | ||
|
|
||
| @override |
There was a problem hiding this comment.
_pulseController repeats continuously for every ScreenShareThumbnail, even when the tile is off-screen/not hovered/not selected. With many participants this can cause unnecessary rebuilds and GPU/CPU usage. Consider only running the animation when needed (e.g., while hovered or while unselected) or replacing it with an implicit animation that doesn’t tick continuously.
| )..repeat(reverse: true); | |
| _pulseAnim = Tween<double>(begin: 0.6, end: 1.0).animate( | |
| CurvedAnimation(parent: _pulseController, curve: Curves.easeInOut), | |
| ); | |
| } | |
| @override | |
| ); | |
| _pulseAnim = Tween<double>(begin: 0.6, end: 1.0).animate( | |
| CurvedAnimation(parent: _pulseController, curve: Curves.easeInOut), | |
| ); | |
| } | |
| @override | |
| void didChangeDependencies() { | |
| super.didChangeDependencies(); | |
| final bool tickerEnabled = TickerMode.of(context); | |
| if (tickerEnabled) { | |
| if (!_pulseController.isAnimating) { | |
| _pulseController.repeat(reverse: true); | |
| } | |
| } else { | |
| if (_pulseController.isAnimating) { | |
| _pulseController.stop(); | |
| } | |
| } | |
| } | |
| @override |
| // Promote this to primary | ||
| ref | ||
| .read(selectedScreenSharesProvider.notifier) | ||
| .clear(); | ||
| ref | ||
| .read(selectedScreenSharesProvider.notifier) | ||
| .select(id); | ||
| // Re-add others after the new primary | ||
| ref | ||
| .read(selectedScreenSharesProvider.notifier) | ||
| .select(primaryId); | ||
| for (final other in secondaryIds) { | ||
| if (other != id) { | ||
| ref | ||
| .read(selectedScreenSharesProvider.notifier) | ||
| .select(other); | ||
| } | ||
| } |
There was a problem hiding this comment.
onToggleExpand promotes a secondary stream by issuing multiple sequential writes to selectedScreenSharesProvider (clear/select/select/loop). This can trigger several rebuilds per click and makes ordering logic harder to reason about. Consider adding a single notifier method that replaces the selection set in one assignment (e.g., setSelectionInOrder([...])) and call that once here.
| // Promote this to primary | |
| ref | |
| .read(selectedScreenSharesProvider.notifier) | |
| .clear(); | |
| ref | |
| .read(selectedScreenSharesProvider.notifier) | |
| .select(id); | |
| // Re-add others after the new primary | |
| ref | |
| .read(selectedScreenSharesProvider.notifier) | |
| .select(primaryId); | |
| for (final other in secondaryIds) { | |
| if (other != id) { | |
| ref | |
| .read(selectedScreenSharesProvider.notifier) | |
| .select(other); | |
| } | |
| } | |
| // Promote this to primary in a single, ordered update | |
| final notifier = | |
| ref.read(selectedScreenSharesProvider.notifier); | |
| final orderedSelection = <String>[ | |
| id, | |
| primaryId, | |
| for (final other in secondaryIds) | |
| if (other != id) other, | |
| ]; | |
| notifier.setSelectionInOrder(orderedSelection); |
No description provided.