fix: restore UPnP/Cast button to now playing screen#189
Conversation
Upstream dropped the CastButton import and call site during the UI reorganization. Re-added to the top-right icon row alongside lyrics, speed, and sleep timer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracted from now_playing_screen so it can be used as a standalone widget. Handles platform routing: AirPlay on iOS, UPnP + Chromecast on Android/other. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a cast/DLNA playback control button to the now-playing screen. The new ChangesCast Button Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/widgets/cast_button.dart`:
- Around line 41-55: The new hardcoded user-facing strings in the CastButton
widget (tooltip assignments, unknown device labels like "device", DLNA prefix
"DLNA:", and other literals such as "Chromecast" / disconnect / playback
messages referenced around the cast-related branches) must be replaced with
localized strings from AppLocalizations; update the tooltip assignments in the
castState connecting/connected/default branches to call
AppLocalizations.of(context)! for the connecting/castDlnaBeta messages and add
localized entries for unknown-device labels and the DLNA prefix, and similarly
swap any other hardcoded literals referenced via CastService.deviceName,
UpnpService.connectedDevice?.friendlyName, and the other occurrences noted
(around the cast/disconnect/playback strings) to use AppLocalizations keys so
all user-facing text is localized.
- Around line 560-579: The dialog is using its own (soon-to-be-disposed) context
for Snackbar: after Navigator.pop(context) the async connectToDevice(d)
completes but ScaffoldMessenger.of(context) refers to the dialog context and the
snackbar never appears; change the dialog to accept and store a parent
BuildContext (e.g. parentContext) passed from the caller and, after calling
Navigator.pop(context), use await widget.castService.connectToDevice(d) as
before but call ScaffoldMessenger.of(parentContext).showSnackBar(...) (and
parentContext.mounted checks where appropriate) so the snackbar is shown on the
parent scaffold; apply the same pattern for the DLNA/UPnP tile logic that also
calls widget.castService.connectToDevice and uses ScaffoldMessenger.of(context).
- Around line 459-460: The current check only compares lengths (devices.length
!= _upnpDevices.length) so replacements with same count are ignored; change the
condition to compare contents using listEquals from flutter/foundation (e.g., if
(!listEquals(devices, _upnpDevices))) and call setState(() => _upnpDevices =
List.of(devices)); ensuring you import foundation.dart if missing and reference
the existing _upnpDevices and devices variables and the setState call in
cast_button.dart.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c93ec406-66e0-41a2-890a-e94a92e206e2
📒 Files selected for processing (2)
lib/screens/now_playing_screen.dartlib/widgets/cast_button.dart
| 'Casting to ${context.read<CastService>().deviceName ?? "device"}'; | ||
| } else if (castState == CastState.connecting) { | ||
| icon = Icons.cast_connected; | ||
| color = Colors.orange; | ||
| tooltip = AppLocalizations.of(context)!.connecting; | ||
| } else if (upnpConnected) { | ||
| icon = Icons.speaker_rounded; | ||
| color = Theme.of(context).colorScheme.primary; | ||
| tooltip = | ||
| 'DLNA: ${context.read<UpnpService>().connectedDevice?.friendlyName ?? "device"}'; | ||
| } else { | ||
| icon = Icons.cast; | ||
| color = iconColor ?? Colors.white; | ||
| tooltip = AppLocalizations.of(context)!.castDlnaBeta; | ||
| } |
There was a problem hiding this comment.
Localize newly introduced user-facing strings.
Several new strings are hardcoded (e.g., casting/DLNA tooltips, unknown device labels, disconnect label, playback message, “Chromecast”). These should go through AppLocalizations to avoid partial-English UI.
Also applies to: 100-101, 266-267, 310-311, 364-365, 558-558
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/widgets/cast_button.dart` around lines 41 - 55, The new hardcoded
user-facing strings in the CastButton widget (tooltip assignments, unknown
device labels like "device", DLNA prefix "DLNA:", and other literals such as
"Chromecast" / disconnect / playback messages referenced around the cast-related
branches) must be replaced with localized strings from AppLocalizations; update
the tooltip assignments in the castState connecting/connected/default branches
to call AppLocalizations.of(context)! for the connecting/castDlnaBeta messages
and add localized entries for unknown-device labels and the DLNA prefix, and
similarly swap any other hardcoded literals referenced via
CastService.deviceName, UpnpService.connectedDevice?.friendlyName, and the other
occurrences noted (around the cast/disconnect/playback strings) to use
AppLocalizations keys so all user-facing text is localized.
| if (devices.length != _upnpDevices.length) { | ||
| setState(() => _upnpDevices = List.of(devices)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '450,470p'Repository: dddevid/Musly
Length of output: 658
🏁 Script executed:
grep -n "_upnpDevices" lib/widgets/cast_button.dartRepository: dddevid/Musly
Length of output: 319
🏁 Script executed:
head -30 lib/widgets/cast_button.dartRepository: dddevid/Musly
Length of output: 955
🏁 Script executed:
rg "class UpnpDevice" -A 20Repository: dddevid/Musly
Length of output: 1206
🏁 Script executed:
rg "List<UpnpDevice>" lib/services/upnp_service.dart -A 5 -B 5Repository: dddevid/Musly
Length of output: 1156
🏁 Script executed:
rg "Future<List<UpnpDevice>> discover" -A 50 lib/services/upnp_service.dartRepository: dddevid/Musly
Length of output: 1634
🏁 Script executed:
rg "\.discover\(" lib/ -B 3 -A 1Repository: dddevid/Musly
Length of output: 337
🏁 Script executed:
rg "_fetchDeviceDescription" -A 30 lib/services/upnp_service.dart | head -50Repository: dddevid/Musly
Length of output: 1578
🌐 Web query:
flutter listEquals foundation.dart documentation
💡 Result:
In Flutter/Dart, listEquals is documented as a function in the foundation library. It compares two lists for element-by-element equality (same length, same elements in the same order) and returns true when both lists are null; false if only one is null or lengths differ.[1] Signature (from the docs): bool listEquals(List? a, List? b).[1] Behavior details from the docs: 1) Null handling: returns true if both a and b are null.[1] 2) Length/order: if both are non-null, it returns true only if they have the same length and matching members at each index in the same order.[1] 3) Nested collections: if the list elements are maps/lists/sets/other collections, their contents are not compared “deeply” unless the elements’ own == operators handle it.[1] The docs recommend DeepCollectionEquality from the collection package for deep equality.[1] 4) See also in the same doc: setEquals and mapEquals for similar equality behavior for those collection types.[1] Source: https://api.flutter.dev/flutter/foundation/listEquals.html [1].
Citations:
UPnP device refresh misses changes when device count stays the same.
When devices are replaced (e.g., 1 removed + 1 added) without changing total count, the length comparison devices.length != _upnpDevices.length skips the setState, leaving stale tiles in the picker. Since UpnpDevice has no custom equality operator, listEquals will detect the reference changes from newly created instances and properly trigger refresh.
Suggested fix
+import 'package:flutter/foundation.dart' show listEquals;- if (devices.length != _upnpDevices.length) {
+ if (!listEquals(devices, _upnpDevices)) {
setState(() => _upnpDevices = List.of(devices));
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/widgets/cast_button.dart` around lines 459 - 460, The current check only
compares lengths (devices.length != _upnpDevices.length) so replacements with
same count are ignored; change the condition to compare contents using
listEquals from flutter/foundation (e.g., if (!listEquals(devices,
_upnpDevices))) and call setState(() => _upnpDevices = List.of(devices));
ensuring you import foundation.dart if missing and reference the existing
_upnpDevices and devices variables and the setState call in cast_button.dart.
| onTap: () async { | ||
| Navigator.pop(context); | ||
| final ok = await widget.castService.connectToDevice(d); | ||
| if (context.mounted) { | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text( | ||
| ok | ||
| ? AppLocalizations.of( | ||
| context, | ||
| )!.connectedToDevice(d.friendlyName) | ||
| : AppLocalizations.of( | ||
| context, | ||
| )!.failedToConnectDevice(d.friendlyName), | ||
| ), | ||
| backgroundColor: ok ? null : Colors.red, | ||
| duration: const Duration(seconds: 2), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "cast_button.dart" -x cat -n {} | head -650 | tail -100Repository: dddevid/Musly
Length of output: 4625
🏁 Script executed:
wc -l lib/widgets/cast_button.dartRepository: dddevid/Musly
Length of output: 90
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '540,625p'Repository: dddevid/Musly
Length of output: 4181
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '480,560p'Repository: dddevid/Musly
Length of output: 3622
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '450,520p'Repository: dddevid/Musly
Length of output: 2749
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '420,460p'Repository: dddevid/Musly
Length of output: 1454
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '1,100p' | grep -n "class\|_DevicePickerDialog"Repository: dddevid/Musly
Length of output: 110
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '380,430p'Repository: dddevid/Musly
Length of output: 1627
🏁 Script executed:
rg "_DevicePickerDialog\(" lib/widgets/cast_button.dart -B 10 -A 2Repository: dddevid/Musly
Length of output: 777
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | grep -A 30 "class _DeviceTile"Repository: dddevid/Musly
Length of output: 1184
🏁 Script executed:
python3 << 'EOF'
# The flow is:
# 1. showDialog(context: context, builder: (context) => _DevicePickerDialog(...))
# - The builder receives a NEW context (dialog context)
#
# 2. In the StreamBuilder inside the dialog:
# builder: (context, snapshot) { ... onTap: () async { Navigator.pop(context) ...
# - This context is the dialog context (from the showDialog builder)
#
# 3. Navigator.pop(context) removes the dialog from the navigation stack
# - The dialog widget is disposed
# - The dialog context is no longer mounted
#
# 4. After await, context.mounted is checked
# - For the dialog context, this will be FALSE
# - The if block won't execute, so no snackbar shows
#
# The review comment is correct: the snackbar won't show because we're using
# the dialog context which gets disposed after Navigator.pop
# The fix is to pass the parent context (from _showDevicePickerDialog)
# to the dialog widget so it can be used for ScaffoldMessenger
print("Context flow analysis:")
print("✓ Parent context (from _showDevicePickerDialog) is available and valid")
print("✓ Dialog context (from showDialog builder) becomes invalid after Navigator.pop")
print("✓ The code uses context.mounted check but it still fails silently")
print("✓ The suggested fix (pass parent context) is correct")
EOFRepository: dddevid/Musly
Length of output: 349
🏁 Script executed:
cat -n lib/widgets/cast_button.dart | sed -n '503,510p'Repository: dddevid/Musly
Length of output: 449
Pass parent context to dialog for showing snackbars.
After device selection, snackbars won't display because the dialog context is disposed immediately by Navigator.pop(context) before the async operation completes. The context.mounted check prevents crashes but the snackbar is never shown since ScaffoldMessenger uses the unmounted dialog context.
Pass the parent context to the dialog widget and use it for displaying feedback after the device connection attempt. This issue affects both the Chromecast (lines 560–579) and DLNA/UPnP (lines 598–617) device tiles.
Suggested fix
class _DevicePickerDialog extends StatefulWidget {
final bool isDark;
+ final BuildContext hostContext;
final GoogleCastDiscoveryManagerPlatformInterface discoveryManager;
final CastService castService;
final UpnpService upnpService;
const _DevicePickerDialog({
required this.isDark,
+ required this.hostContext,
required this.discoveryManager,
required this.castService,
required this.upnpService,
});
} builder: (context) => _DevicePickerDialog(
isDark: isDark,
+ hostContext: context,
discoveryManager: discoveryManager,
castService: castService,
upnpService: upnpService,
),- if (context.mounted) {
- ScaffoldMessenger.of(context).showSnackBar(
+ if (widget.hostContext.mounted) {
+ ScaffoldMessenger.of(widget.hostContext).showSnackBar(
SnackBar(- if (context.mounted) {
- ScaffoldMessenger.of(context).showSnackBar(
+ if (widget.hostContext.mounted) {
+ ScaffoldMessenger.of(widget.hostContext).showSnackBar(
SnackBar(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/widgets/cast_button.dart` around lines 560 - 579, The dialog is using its
own (soon-to-be-disposed) context for Snackbar: after Navigator.pop(context) the
async connectToDevice(d) completes but ScaffoldMessenger.of(context) refers to
the dialog context and the snackbar never appears; change the dialog to accept
and store a parent BuildContext (e.g. parentContext) passed from the caller and,
after calling Navigator.pop(context), use await
widget.castService.connectToDevice(d) as before but call
ScaffoldMessenger.of(parentContext).showSnackBar(...) (and parentContext.mounted
checks where appropriate) so the snackbar is shown on the parent scaffold; apply
the same pattern for the DLNA/UPnP tile logic that also calls
widget.castService.connectToDevice and uses ScaffoldMessenger.of(context).
1 participant
Summary
CastButtonimport and instantiation innow_playing_screen.dart(2 lines)lib/widgets/cast_button.dartwhich was removed in0b2a0919Background
cast_button.dartwas deleted in commit0b2a0919with the note "CastButton class never instantiated." The class had become unreferenced because its import and call site innow_playing_screen.dartwere accidentally dropped during a prior UI reorganization — not intentionally removed. This PR restores both.The widget provides UPnP/DLNA device discovery, Chromecast connect/disconnect, and AirPlay (iOS) support, all surfaced via a single icon button in the Now Playing top-right icon row alongside lyrics, playback speed, and sleep timer.
Test plan
Summary by CodeRabbit