Swipe-left on rows to delete / unsubscribe (iOS-style)#189
Merged
Conversation
Playlist, radio-station, and podcast rows now respond to a left swipe (endToStart) by triggering their respective destructive action, gated on permissions where applicable. Matches iOS Mail / Apple Music conventions. - Playlist row: bidirectional Dismissible — swipe-right keeps the existing 'queue all songs' behavior; swipe-left confirms and deletes (only when Playlist.canDelete). Falls back to startToEnd-only when canDelete is false. - Radio-station row: now wrapped in a Dismissible only when RadioStation.canDelete; swipe-left confirms and deletes. - Podcast row: wrapped in a Dismissible (always — anyone can unsubscribe); swipe-left confirms and unsubscribes. Refactor: - Extracted the confirm-action and execute-action helpers from the long-press menus into public top-level functions so the swipe handlers and menus share one path. - Added a shared SwipeDestructiveBackground widget (red surface, trash icon, optional label) for the secondaryBackground.
📝 WalkthroughWalkthroughThis PR implements swipe-to-delete gestures with confirmation dialogs across playlists, podcasts, and radio stations. A new ChangesSwipe-to-Delete with Confirmation Flows
Sequence DiagramsequenceDiagram
actor User
participant Dismissible
participant Dialog as Confirmation Dialog
participant Provider as Data Provider
participant Toast as Feedback Toast
User->>Dismissible: Swipe item
Dismissible->>Dialog: Show via confirmDismiss()
Dialog->>User: Display confirmation prompt
User->>Dialog: Confirm
Dialog-->>Dismissible: Return true
Dismissible->>Dismissible: Animate dismissal
Dismissible->>Provider: Call via onDismissed()
Provider->>Provider: Remove from state
Provider->>Toast: Show feedback
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 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: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Trash icon alone — no 'Delete' / 'Unsubscribe' text on the swipe drawer. Simpler and more iOS-Mail-like.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/Runner/Info.plist`:
- Around line 29-32: The LSApplicationQueriesSchemes entry in Info.plist uses
"https:" with a trailing colon which is invalid; open the Info.plist and under
the LSApplicationQueriesSchemes array replace the string value "https:" with the
plain scheme name "https" (remove the colon) so the system will correctly
recognize and whitelist the HTTPS scheme.
In `@lib/ui/screens/podcasts.dart`:
- Around line 108-111: The unsubscribe flow must perform optimistic removal: in
PodcastProvider.unsubscribePodcast remove the Podcast from the internal list
(_podcasts) and vault (_vault) and call notifyListeners() before awaiting the
network delete() call so the Dismissible is removed immediately; then await
delete('podcasts/${podcast.id}/subscriptions') inside a try/catch and on catch
restore the podcast into _podcasts and _vault, call notifyListeners() again and
rethrow the error. Ensure you reference the unsubscribePodcast method, the
_podcasts and _vault collections, notifyListeners(), and the delete() network
call when making these changes.
🪄 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: f1712492-b4c5-4931-b080-fcb5cfdda092
📒 Files selected for processing (14)
ios/Flutter/AppFrameworkInfo.plistios/Runner.xcodeproj/project.pbxprojios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschemeios/Runner/AppDelegate.swiftios/Runner/Info.plistlib/ui/screens/playlists.dartlib/ui/screens/podcasts.dartlib/ui/screens/radio_stations.dartlib/ui/widgets/playlist_row.dartlib/ui/widgets/podcast_actions_menu.dartlib/ui/widgets/radio_station_actions_menu.dartlib/ui/widgets/swipe_destructive_background.dartlib/ui/widgets/widgets.dartpubspec.yaml
💤 Files with no reviewable changes (1)
- ios/Flutter/AppFrameworkInfo.plist
Comment on lines
+29
to
+32
| <key>LSApplicationQueriesSchemes</key> | ||
| <array> | ||
| <string>https:</string> | ||
| </array> |
There was a problem hiding this comment.
Remove the trailing colon from the query scheme.
LSApplicationQueriesSchemes expects plain scheme names, so https: will not match and the whitelist entry is ineffective.
Suggested fix
<key>LSApplicationQueriesSchemes</key>
<array>
- <string>https:</string>
+ <string>https</string>
</array>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <key>LSApplicationQueriesSchemes</key> | |
| <array> | |
| <string>https:</string> | |
| </array> | |
| <key>LSApplicationQueriesSchemes</key> | |
| <array> | |
| <string>https</string> | |
| </array> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Runner/Info.plist` around lines 29 - 32, The LSApplicationQueriesSchemes
entry in Info.plist uses "https:" with a trailing colon which is invalid; open
the Info.plist and under the LSApplicationQueriesSchemes array replace the
string value "https:" with the plain scheme name "https" (remove the colon) so
the system will correctly recognize and whitelist the HTTPS scheme.
Comment on lines
+108
to
+111
| onDismissed: (_) => unsubscribePodcastWithFeedback( | ||
| context, | ||
| podcast: podcast, | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
provider_file="$(fd -i 'podcast_provider.dart' lib/providers | head -n1)"
if [[ -z "${provider_file:-}" ]]; then
echo "podcast_provider.dart not found under lib/providers"
exit 1
fi
echo "Inspecting: $provider_file"
rg -nP -C6 'Future<\s*void\s*>\s*unsubscribePodcast\s*\(' "$provider_file"
rg -nP -C4 '_podcasts\.remove|notifyListeners|await|delete\(' "$provider_file"Repository: koel/player
Length of output: 2085
Fix podcast dismissal by using optimistic removal instead of awaiting the network call first.
The unsubscribePodcast implementation in PodcastProvider awaits the network delete before removing from the local list. This breaks the Dismissible contract: when onDismissed fires, the item must be removed from the source list immediately. A rebuild triggered by notifyListeners() after the network call could occur while the Dismissible is still animating, causing a "dismissed widget still part of the tree" error.
Remove the item from _podcasts and call notifyListeners() before the await delete(). If the network call fails, restore the item:
Future<void> unsubscribePodcast(Podcast podcast) async {
_podcasts.remove(podcast);
_vault.remove(podcast.id);
notifyListeners(); // UI updates immediately
try {
await delete('podcasts/${podcast.id}/subscriptions');
} catch (e) {
// Restore on network failure
_podcasts.add(podcast);
_vault[podcast.id] = podcast;
notifyListeners();
rethrow;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ui/screens/podcasts.dart` around lines 108 - 111, The unsubscribe flow
must perform optimistic removal: in PodcastProvider.unsubscribePodcast remove
the Podcast from the internal list (_podcasts) and vault (_vault) and call
notifyListeners() before awaiting the network delete() call so the Dismissible
is removed immediately; then await
delete('podcasts/${podcast.id}/subscriptions') inside a try/catch and on catch
restore the podcast into _podcasts and _vault, call notifyListeners() again and
rethrow the error. Ensure you reference the unsubscribePodcast method, the
_podcasts and _vault collections, notifyListeners(), and the delete() network
call when making these changes.
These files are TestFlight build state + an in-progress Apple-migration local change set the user has been carrying as unstaged. They leaked into this branch when I resolved a stash/pull conflict with 'git checkout --theirs ios/Runner/Info.plist'. Reverting to master so the PR is just the swipe-to-delete change set. The trailing-colon issue in LSApplicationQueriesSchemes (https: vs https) flagged in review still exists on master but is out of scope for this PR.
Address review: a Dismissible's onDismissed callback expects the source list to drop the item synchronously — awaiting the network delete before mutating left the dismissed widget in the tree during the animation, which can throw 'dismissed widget still part of the tree'. Mirror PlaylistProvider.remove's pattern: remove locally + notify, then await the delete, restore on failure.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/providers/podcast_provider.dart`:
- Around line 47-54: Before performing the optimistic removal, record the
podcast's current index from _podcasts (e.g., store in a local variable like
removedIndex); then keep the current try/catch logic but in the catch block
restore the podcast using _podcasts.insert(removedIndex, podcast) instead of
_podcasts.add(podcast), continue to restore _vault[podcast.id] = podcast and
call notifyListeners(), and rethrow; this ensures the item is restored to its
original position when delete('podcasts/${podcast.id}/subscriptions') fails.
🪄 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: fdf50069-4489-4cc2-a45a-1c3360c919ff
📒 Files selected for processing (1)
lib/providers/podcast_provider.dart
Comment on lines
+47
to
+54
| try { | ||
| await delete('podcasts/${podcast.id}/subscriptions'); | ||
| } catch (_) { | ||
| _podcasts.add(podcast); | ||
| _vault[podcast.id] = podcast; | ||
| notifyListeners(); | ||
| rethrow; | ||
| } |
There was a problem hiding this comment.
Rollback restores the podcast at the wrong list position
_podcasts.add(podcast) on line 50 always appends to the end of the list. When the network call fails and the item is restored, it reappears at the bottom instead of its original position, which is visually confusing to the user.
Capture the index before the optimistic removal and use insert during rollback:
🛡️ Proposed fix to preserve list position on rollback
Future<void> unsubscribePodcast(Podcast podcast) async {
// Optimistic removal so a Dismissible's onDismissed callback can
// call this without leaving the dismissed widget in the tree
// while the network call is in flight. Restore on failure.
+ final index = _podcasts.indexOf(podcast);
_podcasts.remove(podcast);
_vault.remove(podcast.id);
notifyListeners();
try {
await delete('podcasts/${podcast.id}/subscriptions');
} catch (_) {
- _podcasts.add(podcast);
+ if (index >= 0) {
+ _podcasts.insert(index, podcast);
+ } else {
+ _podcasts.add(podcast);
+ }
_vault[podcast.id] = podcast;
notifyListeners();
rethrow;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/providers/podcast_provider.dart` around lines 47 - 54, Before performing
the optimistic removal, record the podcast's current index from _podcasts (e.g.,
store in a local variable like removedIndex); then keep the current try/catch
logic but in the catch block restore the podcast using
_podcasts.insert(removedIndex, podcast) instead of _podcasts.add(podcast),
continue to restore _vault[podcast.id] = podcast and call notifyListeners(), and
rethrow; this ensures the item is restored to its original position when
delete('podcasts/${podcast.id}/subscriptions') fails.
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
Adds a left swipe (
endToStart) on the playlist, radio-station, and podcast rows that triggers the destructive action — confirmed via dialog, gated on permissions where applicable. Matches the iOS Mail / Apple Music swipe-to-destroy pattern.Playlist.canDeleteRadioStation.canDeleteThe playlist row is bidirectional: swipe-right keeps the existing "queue all songs" behavior (returning
falsefromconfirmDismissso it bounces back), and swipe-left is the new destructive path. WhencanDeleteisfalse, onlystartToEndis allowed so the queue gesture still works.Refactor
confirmDeletePlaylist/deletePlaylistWithFeedback, same shape for radio + podcast). The long-press menus and the new swipe handlers now share one path — confirm inconfirmDismiss, execute inonDismissed.SwipeDestructiveBackgroundwidget (red surface, trash icon, optional label) used as thesecondaryBackgroundon all three rows.Test plan
flutter test— full suite green (269/269); existing long-press tests still pass against the refactored helpersflutter analyzeclean on touched filescanDelete), a radio station (with/withoutcanDelete), and a podcast — confirm the dialog → toast → row removal flow on eachSummary by CodeRabbit
New Features
Chores