Merged
Conversation
This commit introduces a new feature to the expanded player screen, replacing the static album art with an interactive, swipeable carousel. Key changes include: - Added the `androidx.compose.material3:material3-expressive` dependency to support the new carousel component. - Implemented `AlbumCarouselSection`, a new composable that uses `HorizontalMultiBrowseCarousel` to display the album art of songs in the current playback queue. - Integrated the carousel into the `FullPlayerContentInternal` composable, replacing the previous static album art display. - Implemented a two-way state synchronization between the carousel's state and the `PlayerViewModel` to ensure the UI and playback state are always in sync. - Added "squash" and "fade" animations to the carousel items using the `graphicsLayer` modifier, providing a more expressive and fluid user experience.
…tation
This commit replaces the `HorizontalMultiBrowseCarousel` from `androidx.compose.material3:material3-expressive` with a custom-built, standalone `RoundedHorizontalMultiBrowseCarousel`. This change was motivated by the desire to implement a rounded clipping mask that respects the carousel's item transformations, preventing the "flat edge" artifact seen during peek and scroll animations.
The new implementation is a self-contained version of the Material 3 carousel logic, refactored to allow for a custom `Shape` to be used for clipping at the item level.
Key changes:
- **`RoundedParallaxCarousell.kt`**: New file containing the custom carousel implementation, including `CarouselState`, `RoundedHorizontalMultiBrowseCarousel`, and the underlying logic for keylines, snapping, and layout strategy.
- **`AlbumCarouselSection.kt`**: A new, dedicated composable that encapsulates the carousel logic for the player. It manages the state synchronization between the carousel and the `PlayerViewModel`.
- **`UnifiedPlayerSheet.kt`**:
- Integrated `AlbumCarouselSection` to display the swipeable album art.
- Simplified the `FullPlayerContentInternal` layout by delegating carousel logic.
- Corrected a bug where selecting a song from the carousel did not correctly update the playback queue.
- **`OptimizedAlbumArt.kt`**: Simplified the composable by removing animation and layout logic (`expansionFraction`, `graphicsLayer`), which is now handled by the parent carousel.
- **Dependency Removal**: The `androidx.compose.material3:material3-expressive` dependency has been removed.
- **Code Cleanup**: Removed `@OptIn(ExperimentalMaterial3ExpressiveApi::class)` annotations and updated typography calls from obsolete "Emphasized" styles across several components.
This commit addresses two critical race conditions in the PlayerViewModel that caused state inconsistencies between the UI and the MediaController.
1. **Queue modifications were not persisting:** Actions like reordering or deleting songs in the `QueueBottomSheet` would appear to work momentarily before the UI reverted to the original queue order. This was caused by optimistic UI updates that were subsequently overridden by the player's true state.
The fix removes optimistic UI updates and establishes the `MediaController`'s `onTimelineChanged` listener as the single source of truth for the playback queue. The UI now updates only when the player's timeline changes, ensuring perfect consistency.
2. **Carousel desynchronization:** Rapidly swiping the full-player carousel could cause a state where the displayed album art did not match the currently playing song. This was due to a race condition where slow-to-process transition updates could overwrite the state of the final, correct song.
The fix wraps the `onMediaItemTransition` logic in a cancellable coroutine `Job`. This ensures that when multiple transitions are triggered in quick succession, only the most recent one is processed, preventing state corruption. The `onTimelineChanged` listener also cancels this job to prevent stale data from being used during queue updates.
These changes result in a more robust and predictable player, where the UI state is always synchronized with the `MediaController`'s ground truth.
This commit addresses two critical race conditions in the PlayerViewModel that caused state inconsistencies between the UI and the MediaController.
1. **Queue modifications were not persisting:** Actions like reordering or deleting songs in the `QueueBottomSheet` would appear to work momentarily before the UI reverted to the original queue order. This was caused by optimistic UI updates that were subsequently overridden by the player's true state.
The fix removes optimistic UI updates and establishes the `MediaController`'s `onTimelineChanged` listener as the single source of truth for the playback queue. The UI now updates only when the player's timeline changes, ensuring perfect consistency.
2. **Carousel desynchronization:** Rapidly swiping the full-player carousel could cause a state where the displayed album art did not match the currently playing song. This was due to a race condition where slow-to-process transition updates could overwrite the state of the final, correct song.
The fix wraps the `onMediaItemTransition` logic in a cancellable coroutine `Job`. This ensures that when multiple transitions are triggered in quick succession, only the most recent one is processed, preventing state corruption. The `onTimelineChanged` listener also cancels this job to prevent stale data from being used during queue updates.
This commit also corrects a syntax error where a function definition was incomplete, ensuring the code compiles and functions as intended.
These changes result in a more robust and predictable player, where the UI state is always synchronized with the `MediaController`'s ground truth.
This commit provides a robust fix for two critical state management issues in the PlayerViewModel: the inability to persist queue modifications and the desynchronization of the carousel with the currently playing song. The root cause was identified as the `showAndPlaySong` function improperly resetting the entire playback queue (`MediaItem` list) during navigation actions (e.g., swiping the carousel), instead of simply changing the current track. The fix refactors `showAndPlaySong` to be context-aware: - If the selected song is already present in the current playback queue, the function now uses `mediaController.seekToMediaItem()` to navigate to it. This preserves the existing queue, including any user modifications like reordering or deletions. - If the song is not in the current queue, the function correctly treats it as a new playback context and calls `playSongs` to build and set a new queue. This change ensures that navigating within a playlist no longer destroys its state, providing a stable and predictable user experience. The original fix of using a cancellable job in the `Player.Listener` remains in place to prevent race conditions during rapid transitions.
This commit provides a robust fix for two critical state management issues in the PlayerViewModel: the inability to persist queue modifications and the desynchronization of the carousel with the currently playing song. The root cause was identified as the `showAndPlaySong` function improperly resetting the entire playback queue (`MediaItem` list) during navigation actions (e.g., swiping the carousel), instead of simply changing the current track. This was happening because `playSongs` was called, which uses `mediaController.setMediaItems`, destroying the existing queue. The fix refactors `showAndPlaySong` to be context-aware: - If the selected song is already present in the current playback queue, the function now uses `mediaController.seekTo(windowIndex, positionMs)` to navigate to it. This preserves the existing queue, including any user modifications like reordering or deletions. - If the song is not in the current queue, the function correctly treats it as a new playback context and calls `playSongs` to build and set a new queue. This change ensures that navigating within a playlist no longer destroys its state, providing a stable and predictable user experience. The original fix of using a cancellable job in the `Player.Listener` also remains in place to prevent race conditions during rapid transitions.
This commit provides a robust, multi-layered fix for two critical state management issues: the inability to persist queue modifications and the desynchronization of the full-player carousel with the currently playing song. The root cause was twofold: 1. **ViewModel Logic:** The `showAndPlaySong` function was improperly resetting the entire playback queue during navigation actions (e.g., swiping the carousel), instead of simply changing the current track. 2. **UI Implementation:** The carousel's `onSongSelected` callback was directly calling `playSongs`, a function that resets the queue, instead of using the proper navigation logic in the ViewModel. This commit addresses both issues: - **ViewModel:** The `showAndPlaySong` function is now context-aware. If the selected song is already in the current playback queue, it correctly uses `mediaController.seekTo(windowIndex, positionMs)` to navigate, preserving any user modifications (reordering, etc.). It only rebuilds the queue if the song is from a new context. - **UI (`UnifiedPlayerSheet.kt`):** The carousel's `onSongSelected` lambda has been corrected to call the new, intelligent `showAndPlaySong` function. This ensures that UI-driven navigation correctly uses the ViewModel's navigation logic, preventing the queue from being reset. The carousel's data source has also been confirmed to be `currentPlaybackQueue`, ensuring it always displays the correct, up-to-date song order. These changes work together to create a stable and predictable player where the UI and the `MediaController`'s state are always synchronized, and user modifications to the queue are respected across all interactions.
This commit provides a robust, multi-layered fix for two critical state management issues: the inability to persist queue modifications and the desynchronization of the full-player carousel with the currently playing song, which previously resulted in an infinite loop. The root cause was twofold: 1. **ViewModel Logic:** The `showAndPlaySong` function was improperly resetting the entire playback queue (`MediaItem` list) during navigation actions (e.g., swiping the carousel), instead of simply changing the current track. 2. **UI Feedback Loop:** The carousel UI in `AlbumCarouselSelection.kt` had a `LaunchedEffect` that would re-trigger a song selection after a programmatic scroll, creating an infinite loop between the UI and the player state. This commit addresses both issues: - **ViewModel:** The `showAndPlaySong` function is now context-aware. If the selected song is already in the current playback queue, it correctly uses `mediaController.seekTo(windowIndex, positionMs)` to navigate, preserving any user modifications. It only rebuilds the queue if the song is from a new context. - **UI (`AlbumCarouselSelection.kt`):** The `LaunchedEffect` responsible for handling user swipes now includes `currentSongIndex` in its key. This forces the effect to restart with the correct, up-to-date state after a song change, definitively breaking the feedback loop. - **UI (`UnifiedPlayerSheet.kt`):** The carousel's `onSongSelected` lambda has been corrected to call the new, intelligent `showAndPlaySong` function, ensuring that UI-driven navigation correctly uses the ViewModel's navigation logic. These changes work together to create a stable and predictable player where the UI and the `MediaController`'s state are always synchronized, and user modifications to the queue are respected across all interactions.
This commit provides a robust, multi-layered fix for several critical state management issues, including the inability to persist queue modifications, carousel desynchronization, an infinite playback loop, and incorrect "first swipe" behavior after a reorder. The root cause was a combination of issues in both the ViewModel and the UI: 1. **ViewModel Logic:** The `showAndPlaySong` function was improperly resetting the entire playback queue during navigation actions. 2. **UI Feedback Loop:** The carousel UI in `AlbumCarouselSelection.kt` had a `LaunchedEffect` that would re-trigger a song selection after a programmatic scroll, creating an infinite loop. 3. **Stale UI State:** The same `LaunchedEffect` was not restarting when the queue was reordered, causing it to use a stale version of the queue for the first user swipe. This commit addresses all of these issues: - **ViewModel:** `showAndPlaySong` is now context-aware. It uses `mediaController.seekTo()` to navigate within an existing queue, preserving all modifications. It only rebuilds the queue for new playback contexts. - **UI (`AlbumCarouselSelection.kt`):** The `LaunchedEffect` responsible for handling user swipes now includes both `currentSongIndex` and the `queue` itself as keys. This forces the effect to restart with the correct, up-to-date state after any song or queue change, definitively breaking the feedback loop and ensuring the correct song order is always used. - **UI (`UnifiedPlayerSheet.kt`):** The carousel's `onSongSelected` lambda has been corrected to call the new, intelligent `showAndPlaySong` function, ensuring that UI-driven navigation correctly uses the ViewModel's navigation logic. These changes work together to create a stable and predictable player where the UI and the `MediaController`'s state are always synchronized, and user modifications to the queue are respected across all interactions.
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.
No description provided.