Replace radio station popup menu with a full action sheet#194
Merged
Conversation
Mirrors the album/artist action sheet pattern. The long-press on radio station cards and rows now opens an Apple-Music-style sheet with thumbnail, name, optional description, a Favorite + Play/Stop quick-action row, and Edit / Delete list rows (gated on canEdit / canDelete). The Play/Stop button is reactive: it shows Stop while this station is the current loading or playing one, otherwise Play. Tapping Play kicks the radio over to this station; Stop stops radio mode. Adds the koel >= 7.11.0-gated favorite toggle on RadioStation + RadioStationProvider, and adds a destructive flag to PlayableActionButton for the red Delete row. The confirmDeleteRadioStation / deleteRadioStationWithFeedback helpers move from the deleted radio_station_actions_menu.dart into a small radio_station_actions.dart so the swipe-to-delete in the list keeps using them.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces the position-based long-press context menu with a bottom-sheet RadioStationActionSheet, adds a ChangesRadio Station Favorite & Action Sheet Refactor
Sequence DiagramsequenceDiagram
participant User
participant Sheet as RadioStationActionSheet
participant StationProv as RadioStationProvider
participant PlayerProv as RadioPlayerProvider
participant UI as Overlay/UI
User->>Sheet: Long-press station → open sheet
Sheet->>User: Render thumbnail and actions
rect rgba(100,150,200,0.5)
User->>Sheet: Tap Favorite / Undo Favorite
Sheet->>StationProv: toggleFavorite(station)
StationProv->>StationProv: Optimistically flip station.favorite
StationProv->>User: notifyListeners() (UI updates)
StationProv->>StationProv: post('favorites/toggle', ...)
StationProv-->>Sheet: success or error (on error revert and notify)
end
rect rgba(100,150,200,0.5)
User->>Sheet: Tap Play or Stop
Sheet->>Sheet: Pop sheet (dismiss)
Sheet->>PlayerProv: play(station) OR stop()
Sheet->>UI: swallow errors (no crash)
end
rect rgba(100,150,200,0.5)
User->>Sheet: Tap Delete
Sheet->>User: Show confirm dialog
User->>Sheet: Confirm
Sheet->>Sheet: Pop sheet
Sheet->>StationProv: remove(station) (fire-and-forget)
Sheet->>UI: show "Station deleted" overlay
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
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)
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. Review rate limit: 6/8 reviews remaining, refill in 10 minutes and 50 seconds.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: 1
🧹 Nitpick comments (1)
lib/ui/screens/radio_station_action_sheet.dart (1)
150-156: 💤 Low value
color: Colors.white30on the destructive Delete icon is dead code.
PlayableActionButtonreplaces the icon entirely with a newIcon(icon.icon, color: destructiveColor)whendestructive: true && enabled: true(seelib/ui/screens/playable_action_sheet.dart), soColors.white30is never rendered. Omitting thecolorargument makes the intent clearer and aligns with how non-destructive icons are already passed in the Edit button (line 137).♻️ Proposed cleanup
- icon: const Icon( - CupertinoIcons.trash, - color: Colors.white30, - ), + icon: const Icon(CupertinoIcons.trash),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/radio_station_action_sheet.dart` around lines 150 - 156, The Delete button passes a colored Icon that is never used because PlayableActionButton replaces the icon when destructive is true; remove the color argument from the Icon passed into PlayableActionButton (i.e., change the Delete button's Icon constructor so it doesn't set color) so the destructiveColor path in PlayableActionButton is used, matching how the Edit button supplies its icon.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/ui/screens/radio_station_action_sheet.dart`:
- Around line 112-122: The onTap handler discards the Future returned by
radioPlayer.stop() which can produce an unhandled async error; update the onTap
closure so that radioPlayer.stop() is invoked with a catchError handler
(matching how radioPlayer.play(station).catchError((_) {}) and toggleFavorite()
are handled) after Navigator.pop(context) to swallow errors safely; locate the
onTap block that calls Navigator.pop(context) and replace the bare
radioPlayer.stop() call with a call that appends .catchError((_) {}) to prevent
unhandled exceptions.
---
Nitpick comments:
In `@lib/ui/screens/radio_station_action_sheet.dart`:
- Around line 150-156: The Delete button passes a colored Icon that is never
used because PlayableActionButton replaces the icon when destructive is true;
remove the color argument from the Icon passed into PlayableActionButton (i.e.,
change the Delete button's Icon constructor so it doesn't set color) so the
destructiveColor path in PlayableActionButton is used, matching how the Edit
button supplies its icon.
🪄 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: 24cd01d3-cd04-486d-8977-371b9c26c263
📒 Files selected for processing (14)
lib/models/radio_station.dartlib/providers/radio_station_provider.dartlib/ui/screens/playable_action_sheet.dartlib/ui/screens/radio_station_action_sheet.dartlib/ui/screens/radio_stations.dartlib/ui/screens/screens.dartlib/ui/widgets/radio_station_actions.dartlib/ui/widgets/radio_station_actions_menu.dartlib/ui/widgets/radio_station_card.dartlib/ui/widgets/widgets.darttest/models/radio_station_test.darttest/ui/screens/radio_station_action_sheet_test.darttest/ui/screens/radio_station_action_sheet_test.mocks.darttest/ui/widgets/radio_station_card_test.dart
💤 Files with no reviewable changes (2)
- test/ui/widgets/radio_station_card_test.dart
- lib/ui/widgets/radio_station_actions_menu.dart
Editing the URL of the station that's currently on air didn't change what the listener heard: setUrl was only called at play time on a proxy URL keyed by id, and the player kept pulling bytes through the existing connection. Editing only the name didn't refresh the OS media-session metadata either, so the lock screen / notification kept showing the old name until the next now-playing poll happened to push a new streamTitle. Add RadioPlayerProvider.refreshMediaItem(). The edit sheet captures the old name + URL, and when the edited station is the current one, restarts the stream on URL change, otherwise refreshes the media item on name change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mirrors the album/artist action sheet pattern (#190 / #191) for radio stations. Long-pressing a radio station card or row now opens an Apple-Music-style sheet with:
Adds the optimistic favorite toggle on `RadioStation` + `RadioStationProvider` (POSTs `favorites/toggle` with `type: 'radio-station'`), and adds a `destructive` flag to `PlayableActionButton` so the Delete row can render in red.
The old `radio_station_actions_menu.dart` is gone. The `confirmDeleteRadioStation` / `deleteRadioStationWithFeedback` helpers move into a small `radio_station_actions.dart` so the swipe-to-delete in the list keeps using them.
Test plan
Summary by CodeRabbit
New Features
Refactor
Tests