Inline queue in Now Playing screen#148
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSubscribed to Changes
Sequence Diagram(s)sequenceDiagram
participant UI as NowPlayingScreen
participant Provider as PlayableProvider
participant Audio as AudioHandler
UI->>Audio: subscribe to queue stream
Audio-->>UI: queue update (List<MediaItem>)
UI->>Provider: byId(id) for each MediaItem
Provider-->>UI: Playable or null
UI->>UI: build _queuePlayables (filter nulls) & setState
alt user swipes a queue row (end-to-start)
UI->>Audio: removeQueueItemAt(index)
Audio-->>UI: updated queue stream
end
alt user reorders a queue row
UI->>Audio: moveQueueItem(fromIndex, toIndex)
Audio-->>UI: updated queue stream
end
alt user overscrolls to dismiss screen
UI->>UI: NotificationListener accumulates _dragOffset/_isDismissing
UI-->>UI: pop route when threshold met or reset offset
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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. 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 the current code and only fix it if needed.
Inline comments:
In `@lib/ui/screens/now_playing.dart`:
- Around line 275-279: Rows use ValueKey(_queuePlayables[index]) which collides
when the same Playable object (or id) appears multiple times; change the key for
PlayableRow (and any surrounding Dismissible/reorderable child) to a per-entry
unique key — e.g., combine the playable id with the queue position or use the
queue entry's unique id (e.g., "${playable.id}-$index" or an ObjectKey built
from the entry) so each queue entry has a distinct key and row state/reordering
works correctly.
- Around line 50-57: The current subscriber filters out nulls from
_playableProvider.byId which shifts indices; change the state to preserve null
placeholders so indices remain aligned with audioHandler.queue: update
_queuePlayables to be List<Playable?> and in the subscribe callback set
_queuePlayables = value.map((item) => _playableProvider.byId(item.id)).toList()
(remove the .where/.cast). Also audit any dismiss/reorder handlers (the
callbacks that use _queuePlayables indices) to tolerate nullable entries (guard
before using a Playable) so they act on the correct queue index even when the
Playable is unresolved.
- Around line 114-116: The GestureDetector wrapping the view (handlers:
_onVerticalDragUpdate and _onVerticalDragEnd) competes with the inner
CustomScrollView for vertical drag gestures; replace or refactor this pattern to
use Flutter's DraggableScrollableSheet so the sheet and its scrollable child
share gesture/scroll coordination and a shared ScrollController, or if you must
keep GestureDetector, implement coordinated gesture handling by wiring a
ScrollController to CustomScrollView and conditionally delegating vertical drags
(e.g., only trigger _onVerticalDrag* when the scroll position is at the top and
the drag is downward); update references in the now_playing.dart widget to use
DraggableScrollableSheet (or the conditional delegation) and ensure handlers
_onVerticalDragUpdate/_onVerticalDragEnd are removed or adapted accordingly.
🪄 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: 5a098579-ba99-4ca6-9abf-99e3517c675e
📒 Files selected for processing (1)
lib/ui/screens/now_playing.dart
Comment on lines
+50
to
+57
| subscribe(audioHandler.queue.listen((List<MediaItem> value) { | ||
| setState(() { | ||
| _queuePlayables = value | ||
| .map((item) => _playableProvider.byId(item.id)) | ||
| .where((p) => p != null) | ||
| .cast<Playable>() | ||
| .toList(); | ||
| }); |
There was a problem hiding this comment.
Don’t filter queue entries out of state.
PlayableProvider.byId() can return null here. Because this widget only refreshes when audioHandler.queue emits, any unresolved MediaItem disappears from the inline queue until the queue changes again. That also shifts the rendered indices away from the real queue indices, so the later dismiss/reorder callbacks can mutate the wrong item.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ui/screens/now_playing.dart` around lines 50 - 57, The current subscriber
filters out nulls from _playableProvider.byId which shifts indices; change the
state to preserve null placeholders so indices remain aligned with
audioHandler.queue: update _queuePlayables to be List<Playable?> and in the
subscribe callback set _queuePlayables = value.map((item) =>
_playableProvider.byId(item.id)).toList() (remove the .where/.cast). Also audit
any dismiss/reorder handlers (the callbacks that use _queuePlayables indices) to
tolerate nullable entries (guard before using a Playable) so they act on the
correct queue index even when the Playable is unresolved.
Comment on lines
+275
to
+279
| key: ValueKey(_queuePlayables[index]), | ||
| child: PlayableRow( | ||
| index: index, | ||
| key: ValueKey(_queuePlayables[index]), | ||
| playable: _queuePlayables[index], |
There was a problem hiding this comment.
These row keys collide when the same track is queued twice.
PlayableProvider.byId() returns the same cached Playable for a given id, so two queue entries for the same song end up with identical ValueKeys here. Dismissible and reorderable children need unique per-entry keys; otherwise you can get assertions or row state being reused for the wrong item.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ui/screens/now_playing.dart` around lines 275 - 279, Rows use
ValueKey(_queuePlayables[index]) which collides when the same Playable object
(or id) appears multiple times; change the key for PlayableRow (and any
surrounding Dismissible/reorderable child) to a per-entry unique key — e.g.,
combine the playable id with the queue position or use the queue entry's unique
id (e.g., "${playable.id}-$index" or an ObjectKey built from the entry) so each
queue entry has a distinct key and row state/reordering works correctly.
phanan
force-pushed
the
feat/inline-queue-now-playing
branch
4 times, most recently
from
ea7934f to
f9f8456
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
lib/ui/screens/now_playing.dart (2)
33-33:⚠️ Potential issue | 🟠 MajorKeep the inline queue state 1:1 with
audioHandler.queue.
PlayableProvider.byId()is nullable, and filtering misses out here makes unresolved items disappear until the queue mutates again. It also shifts rendered indices away from the real queue, so Line 269 and Line 288 can remove or reorder the wrong entry becauseremoveQueueItemAt()/moveQueueItem()index into the full queue (lib/audio_handler.dart:295-310). Preserve nullable placeholders or keep the underlyingMediaItemalongside the resolvedPlayable.Also applies to: 50-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` at line 33, The inline queue stored in _queuePlayables is being filtered by PlayableProvider.byId(), dropping nulls and desynchronizing indices from audioHandler.queue; change the construction/updates of _queuePlayables to preserve placeholders for unresolved entries (e.g., keep null or a sentinel for entries where PlayableProvider.byId(id) returns null) or maintain a parallel list of the original MediaItem entries so the index mapping is 1:1 with audioHandler.queue; ensure removeQueueItemAt() and moveQueueItem() continue to index into the full queue (audioHandler.queue) while using the preserved placeholders or MediaItem list to render items and resolve Playable lazily.
279-283:⚠️ Potential issue | 🟠 MajorUse a per-entry key for both queue widgets.
ValueKey(_queuePlayables[index])collides as soon as the same track is queued twice, soDismissibleandPlayableRowcan reuse state for the wrong row or hit duplicate-key assertions. Once you keep the actual queue entry in state, key both widgets from that entry instead of the resolvedPlayable. Quick repro: queue the same song twice, then dismiss/reorder both rows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 279 - 283, The current keys use ValueKey(_queuePlayables[index]) which collides when the same Playable is queued twice; update both the Dismissible and the PlayableRow to use the unique queue entry object (or its unique id) as the key instead of the resolved Playable. Locate where Dismissible (the parent) and PlayableRow are instantiated and replace ValueKey(_queuePlayables[index]) with a per-entry key such as ValueKey(queueEntry) or ValueKey(queueEntry.id) using the queue entry variable you hold in state (the same entry reference for both widgets) so each queued instance has a unique key.
🧹 Nitpick comments (1)
lib/ui/screens/now_playing.dart (1)
137-143: Prefer a minimum height here instead of a fixed height.The controls section is hard-capped at
screenHeight - 80, so on short viewports or larger text sizes theColumncan overflow even though the page itself scrolls. Using a minimum height keeps the peek effect on normal screens but still lets the controls grow when needed. Quick check: test this on a small device / landscape with larger text scaling.Suggested tweak
- child: Container( - // Show the queue header + half of the first song peeking - // when the queue has items; full height when empty. - height: _queuePlayables.isEmpty - ? screenHeight - : screenHeight - 80, + child: Container( + // Show the queue header + half of the first song peeking + // when the queue has items; full height when empty. + constraints: BoxConstraints( + minHeight: _queuePlayables.isEmpty + ? screenHeight + : screenHeight - 80, + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 137 - 143, Replace the fixed height on the Container that uses _queuePlayables and screenHeight with a minimum height constraint so the controls can grow on small viewports or large text sizes; specifically, remove the height: ... line in the Container and use constraints (BoxConstraints with minHeight set to _queuePlayables.isEmpty ? screenHeight : screenHeight - 80) so the peek effect remains but the Column can expand when needed while keeping the existing padding and layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/ui/screens/now_playing.dart`:
- Line 33: The inline queue stored in _queuePlayables is being filtered by
PlayableProvider.byId(), dropping nulls and desynchronizing indices from
audioHandler.queue; change the construction/updates of _queuePlayables to
preserve placeholders for unresolved entries (e.g., keep null or a sentinel for
entries where PlayableProvider.byId(id) returns null) or maintain a parallel
list of the original MediaItem entries so the index mapping is 1:1 with
audioHandler.queue; ensure removeQueueItemAt() and moveQueueItem() continue to
index into the full queue (audioHandler.queue) while using the preserved
placeholders or MediaItem list to render items and resolve Playable lazily.
- Around line 279-283: The current keys use ValueKey(_queuePlayables[index])
which collides when the same Playable is queued twice; update both the
Dismissible and the PlayableRow to use the unique queue entry object (or its
unique id) as the key instead of the resolved Playable. Locate where Dismissible
(the parent) and PlayableRow are instantiated and replace
ValueKey(_queuePlayables[index]) with a per-entry key such as
ValueKey(queueEntry) or ValueKey(queueEntry.id) using the queue entry variable
you hold in state (the same entry reference for both widgets) so each queued
instance has a unique key.
---
Nitpick comments:
In `@lib/ui/screens/now_playing.dart`:
- Around line 137-143: Replace the fixed height on the Container that uses
_queuePlayables and screenHeight with a minimum height constraint so the
controls can grow on small viewports or large text sizes; specifically, remove
the height: ... line in the Container and use constraints (BoxConstraints with
minHeight set to _queuePlayables.isEmpty ? screenHeight : screenHeight - 80) so
the peek effect remains but the Column can expand when needed while keeping the
existing padding and layout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbc6e3cd-286d-4e8f-86ff-dfa01f0f5321
📒 Files selected for processing (1)
lib/ui/screens/now_playing.dart
phanan
force-pushed
the
feat/inline-queue-now-playing
branch
2 times, most recently
from
593bc2b to
8093949
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/ui/screens/now_playing.dart (2)
163-165: Consider extracting magic number106to a named constant.The calculation
screenHeight - 106represents the queue header plus partial song preview height. A named constant would improve readability and maintainability.♻️ Suggested change
+ // Height of queue header + half of first visible queue item + static const _queuePeekOffset = 106.0; + `@override` Widget build(BuildContext context) { ... height: _queuePlayables.isEmpty ? screenHeight - : screenHeight - 106, + : screenHeight - _queuePeekOffset,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 163 - 165, Extract the magic number 106 into a clearly named constant (e.g., kQueueHeaderAndPreviewHeight or queueHeaderPreviewHeight) and use that constant in the calculation instead of the literal; update the expression in the widget using _queuePlayables and screenHeight to read screenHeight - kQueueHeaderAndPreviewHeight (or your chosen name) so the intent of "queue header plus partial song preview height" is explicit and maintainable.
76-84: Potential state leak if user scrolls back down during drag-to-dismiss.Once
_isDraggingDownis true, anyScrollUpdateNotificationwith non-nulldragDetailscontinues to update_dragOffset. If the user reverses direction and scrolls back into content (positive delta),_dragOffsetwill accumulate those deltas rather than reset to normal scroll behavior. Consider resetting_isDraggingDown = falseand_dragOffset = 0when the scroll offset becomes positive.♻️ Suggested fix
if (_isDraggingDown && notification is ScrollUpdateNotification) { + // If scrolled back into content, exit drag-to-dismiss mode + if (_scrollController.offset > 0) { + setState(() { + _isDraggingDown = false; + _dragOffset = 0; + }); + return false; + } if (notification.dragDetails != null) { setState(() { _dragOffset = (_dragOffset + notification.dragDetails!.delta.dy) .clamp(0.0, double.infinity); }); return true; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 76 - 84, The ScrollUpdateNotification handler currently only accumulates drag deltas while _isDraggingDown is true causing _dragOffset to keep growing if the user scrolls back into content; update the ScrollUpdateNotification branch (the block checking notification is ScrollUpdateNotification and notification.dragDetails) to detect when the incoming drag delta or resulting scroll offset is positive and in that case set _isDraggingDown = false and _dragOffset = 0 (and call setState) so normal scrolling resumes; keep the existing logic for applying negative deltas when still dragging down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/ui/screens/now_playing.dart`:
- Around line 163-165: Extract the magic number 106 into a clearly named
constant (e.g., kQueueHeaderAndPreviewHeight or queueHeaderPreviewHeight) and
use that constant in the calculation instead of the literal; update the
expression in the widget using _queuePlayables and screenHeight to read
screenHeight - kQueueHeaderAndPreviewHeight (or your chosen name) so the intent
of "queue header plus partial song preview height" is explicit and maintainable.
- Around line 76-84: The ScrollUpdateNotification handler currently only
accumulates drag deltas while _isDraggingDown is true causing _dragOffset to
keep growing if the user scrolls back into content; update the
ScrollUpdateNotification branch (the block checking notification is
ScrollUpdateNotification and notification.dragDetails) to detect when the
incoming drag delta or resulting scroll offset is positive and in that case set
_isDraggingDown = false and _dragOffset = 0 (and call setState) so normal
scrolling resumes; keep the existing logic for applying negative deltas when
still dragging down.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92ff47ba-baa0-4650-a5ae-261c27ed9522
📒 Files selected for processing (1)
lib/ui/screens/now_playing.dart
phanan
force-pushed
the
feat/inline-queue-now-playing
branch
2 times, most recently
from
be0e842 to
385c122
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/ui/screens/now_playing.dart (1)
142-144: Consider extracting magic number 106 as a named constant.The value 106 represents "queue header + half of first song" but is hardcoded. If the queue header or row height changes, this calculation will be incorrect.
♻️ Suggested approach
// At class level or constants file static const _queuePeekHeight = 106.0; // queue header (56) + half row (~50) // In build height: _queuePlayables.isEmpty ? screenHeight : screenHeight - _queuePeekHeight,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 142 - 144, Extract the magic number 106 used in the height calculation into a named constant (e.g., static const double _queuePeekHeight = 106.0) placed at the class level (or a shared constants file) and replace the literal in the ternary expression that computes height (the expression referencing _queuePlayables and screenHeight) with screenHeight - _queuePeekHeight so the meaning ("queue header + half of first song") is explicit and easy to update when row/header sizes change.
🤖 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/now_playing.dart`:
- Around line 267-268: The dismiss handler currently uses the build-time index
(onDismissed -> audioHandler.removeQueueItemAt(index)), which can be wrong if
the queue stream updates during the dismiss animation; instead capture the
identity of the item and use that to remove it and ensure the Dismissible uses a
stable unique key. Concretely: in the widget build, capture final item =
queue[index] (or its unique id) and change onDismissed to remove by item
identity (or find the current index via queue.indexWhere(id matches) before
calling removeQueueItemAt), and update the Dismissible key to use that stable id
rather than the list index to avoid key collisions.
---
Nitpick comments:
In `@lib/ui/screens/now_playing.dart`:
- Around line 142-144: Extract the magic number 106 used in the height
calculation into a named constant (e.g., static const double _queuePeekHeight =
106.0) placed at the class level (or a shared constants file) and replace the
literal in the ternary expression that computes height (the expression
referencing _queuePlayables and screenHeight) with screenHeight -
_queuePeekHeight so the meaning ("queue header + half of first song") is
explicit and easy to update when row/header sizes change.
🪄 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: 4240e8bb-bfee-45d3-9c2a-9774c6a88806
📒 Files selected for processing (1)
lib/ui/screens/now_playing.dart
phanan
force-pushed
the
feat/inline-queue-now-playing
branch
from
385c122 to
298667b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/ui/screens/now_playing.dart (2)
51-57:⚠️ Potential issue | 🟠 MajorKeep queue entries index-aligned with
audioHandler.queue.At Line 53–57, filtering out unresolved
Playables changes list length/order, so UI indices no longer match queue indices used later for remove/reorder operations. This can remove or move the wrong item.Suggested fix
- var _queuePlayables = <Playable>[]; + var _queuePlayables = <Playable?>[]; subscribe(audioHandler.queue.listen((List<MediaItem> value) { setState(() { _queuePlayables = value .map((item) => _playableProvider.byId(item.id)) - .where((p) => p != null) - .cast<Playable>() .toList(); }); }));Then guard nullable entries in row rendering/actions so queue indices remain 1:1 with
audioHandler.queue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 51 - 57, The current mapping in the audioHandler.queue.listen callback filters out null Playable entries (via .where and .cast), breaking index alignment with audioHandler.queue; change the subscription so _queuePlayables captures the same-length list (map to _playableProvider.byId(item.id) and call toList() without filtering) and make _queuePlayables a List<Playable?> (nullable) so null slots are preserved; then update row rendering and action handlers (the widget that reads _queuePlayables by index and functions like remove/reorder) to guard for null (skip render or show placeholder and disable actions) so UI indices remain 1:1 with audioHandler.queue.
274-291:⚠️ Potential issue | 🟠 MajorUse stable per-entry keys and avoid build-time index removal.
At Line 277 and Line 287/Line 290, two issues combine:
ValueKey(_queuePlayables[index])collides when duplicate tracks map to the samePlayable.removeQueueItemAt(index)uses a captured index that can be stale by dismissal time.Suggested fix
itemBuilder: (BuildContext context, int index) { - return Dismissible( + final queueEntry = audioHandler.queue.value[index]; + return Dismissible( direction: DismissDirection.endToStart, onDismissed: (_) async { - await audioHandler.removeQueueItemAt(index); + final currentIndex = audioHandler.queue.value.indexOf(queueEntry); + if (currentIndex >= 0) { + await audioHandler.removeQueueItemAt(currentIndex); + } }, @@ - key: ValueKey(_queuePlayables[index]), + key: ObjectKey(queueEntry), child: PlayableRow( index: index, - key: ValueKey(_queuePlayables[index]), + key: ObjectKey(queueEntry), playable: _queuePlayables[index], listContext: PlayableListContext.queue, ), ); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/screens/now_playing.dart` around lines 274 - 291, The Dismissible and PlayableRow are using unstable keys and a captured build-time index: replace ValueKey(_queuePlayables[index]) with a stable per-item key (e.g. ValueKey(playable.id) or another unique identifier on the Playable) for both the Dismissible and PlayableRow, and stop using the captured index inside the onDismissed closure; instead resolve the current item at dismissal time (e.g. find the current index by searching _queuePlayables or the audioHandler.queue for the playable.id) and then call audioHandler.removeQueueItemAt(foundIndex) or use a remove-by-id/removeQueueItem API if available, referencing Dismissible, PlayableRow, _queuePlayables, ValueKey, and removeQueueItemAt in your changes.
🤖 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/now_playing.dart`:
- Line 296: The UI's onReorder currently calls audioHandler.moveQueueItem which
only mutates queue.value, so update moveQueueItem (in lib/audio_handler.dart) to
also reorder the underlying player's playback sequence (call the player
backend's reorder method or sequence API) so the playback order and displayed
queue stay in sync; perform the player reorder first (or in a
transactional/try/catch) and only emit the updated queue.value after the player
confirms success, and ensure errors roll back or log appropriately so
next/previous behavior matches the inline queue.
---
Duplicate comments:
In `@lib/ui/screens/now_playing.dart`:
- Around line 51-57: The current mapping in the audioHandler.queue.listen
callback filters out null Playable entries (via .where and .cast), breaking
index alignment with audioHandler.queue; change the subscription so
_queuePlayables captures the same-length list (map to
_playableProvider.byId(item.id) and call toList() without filtering) and make
_queuePlayables a List<Playable?> (nullable) so null slots are preserved; then
update row rendering and action handlers (the widget that reads _queuePlayables
by index and functions like remove/reorder) to guard for null (skip render or
show placeholder and disable actions) so UI indices remain 1:1 with
audioHandler.queue.
- Around line 274-291: The Dismissible and PlayableRow are using unstable keys
and a captured build-time index: replace ValueKey(_queuePlayables[index]) with a
stable per-item key (e.g. ValueKey(playable.id) or another unique identifier on
the Playable) for both the Dismissible and PlayableRow, and stop using the
captured index inside the onDismissed closure; instead resolve the current item
at dismissal time (e.g. find the current index by searching _queuePlayables or
the audioHandler.queue for the playable.id) and then call
audioHandler.removeQueueItemAt(foundIndex) or use a remove-by-id/removeQueueItem
API if available, referencing Dismissible, PlayableRow, _queuePlayables,
ValueKey, and removeQueueItemAt in your 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: 69dc596f-ddb6-4002-b38f-1880b289b836
📒 Files selected for processing (1)
lib/ui/screens/now_playing.dart
| ), | ||
| ); | ||
| }, | ||
| onReorder: audioHandler.moveQueueItem, |
There was a problem hiding this comment.
Reorder callback can desync displayed queue from playback order.
At Line 296, onReorder: audioHandler.moveQueueItem delegates to lib/audio_handler.dart Line 304–310, which only mutates queue.value and emits it. If playback sequence is not reordered in the player backend as well, next/previous playback order can diverge from the inline queue order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/ui/screens/now_playing.dart` at line 296, The UI's onReorder currently
calls audioHandler.moveQueueItem which only mutates queue.value, so update
moveQueueItem (in lib/audio_handler.dart) to also reorder the underlying
player's playback sequence (call the player backend's reorder method or sequence
API) so the playback order and displayed queue stay in sync; perform the player
reorder first (or in a transactional/try/catch) and only emit the updated
queue.value after the player confirms success, and ensure errors roll back or
log appropriately so next/previous behavior matches the inline queue.
eda6bf5 to
bebbd3a
Compare
Replace the queue icon button with an inline queue list below the now playing controls. Users can swipe up to see the queue — the full queue is rendered as a SliverReorderableList below the player controls in a single scrollable view. - Reorderable queue with drag handles - Swipe to remove from queue - Clear queue button - Queue header with song count - Removed the queue navigation icon button
phanan
force-pushed
the
feat/inline-queue-now-playing
branch
from
bebbd3a to
b385bae
Compare
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
Test plan
Summary by CodeRabbit
New Features
UI/UX Improvements