fix(player): pass playerBarNavigationAction to sidebar playlist views - #439
Open
tsibog wants to merge 3 commits into
Open
fix(player): pass playerBarNavigationAction to sidebar playlist views#439tsibog wants to merge 3 commits into
tsibog wants to merge 3 commits into
Conversation
viewForSidebarPinnedItem creates PlaylistDetailView without a playerBarNavigationAction, defaulting to .disabled. This makes the artist name and album thumbnail in the player bar non-clickable when playing from a sidebar playlist. Liked Music already passes one (likedMusicPlayerBarNavigationAction); apply the same pattern using the per-item pinnedNavigationPaths binding via .playerBarMusicNavigation. Fixes sozercan#437
Contributor
Author
|
@codex review |
PlaylistDetailView/SimplePlaylistDetailView set the environment on their own PlayerBar, overriding any value set higher up the tree. Passing the action only via .playerBarMusicNavigation left it at .disabled, so artist and album names stayed unclickable when playing from a sidebar playlist. Build the per-item action from pinnedNavigationPaths and pass it into the detail views and navigationDestinations, matching the Liked Music path. Extract the duplicated path binding into a helper.
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.
Description
When playing a track from a sidebar playlist (not Liked Music), the artist and album names in the player bar were not clickable.
viewForSidebarPinnedItemcreatedPlaylistDetailViewwithout aplayerBarNavigationAction, so it defaulted to.disabled. ThePlayerBarinside then sawnavigationAction.openArtist == nil, makingcanOpenCurrentArtistfalse and rendering the artist name disabled.Correction from the first revision of this PR: the fix cannot be done with
.playerBarMusicNavigation(path:)on the enclosingNavigationStackalone.PlaylistDetailViewsets the environment value directly on its ownPlayerBar:That write is closer to the
PlayerBarthan the modifier on the stack, so it wins — and since the view was constructed without the argument, it stayed.disabled. The modifier-only version was verified by inspection but never actually worked when built and run.Liked Music works because it passes the action into the view's initializer (
MainWindow.swift:606), not just via the outer modifier. This PR applies that same shape to sidebar playlists.Type of Change
Related Issues
Fixes #437
Changes Made
pinnedItemPlayerBarNavigationAction(for:), mirroringlikedMusicPlayerBarNavigationActionbut keyed on the per-itemcontentIdpath.PlaylistDetailViewandSimplePlaylistDetailView— this is what actually fixes the bug.navigationDestinations(client:playerBarNavigationAction:)so the player bar stays clickable after pushing to an artist or album page, matching Liked Music (MainWindow.swift:618-620).pinnedItemNavigationPath(for:)for the path binding that was duplicated inline.Testing
swift test --skip KasetUITests) — 2956 tests in 232 suitesswiftlint --strict && swiftformat .cleanVerified in a running packaged build: playing a track from a sidebar playlist and clicking the artist name in the player bar now navigates to the artist page, and Liked Music continues to work.
Checklist
swiftlint --strict && swiftformat .Additional Notes
While testing this, an unrelated pre-existing bug surfaced: the player bar's album artwork flickers on every view switch. It reproduces in released builds and is not caused by this PR — instrumentation showed the whole
PlayerBaris remounted per navigation because each content view constructs its own via.safeAreaInset. Filed separately with the trace data, since the structural fix there would retire this entire class of bug.