feat(ui): make artist names clickable in track list rows - #442
Conversation
Artist names in playlist track rows are now NavigationLinks when the artist has a navigable ID, instead of plain Text. Non-navigable artists remain plain text. Reuses the existing HoverUnderlineNavigationLink pattern with customizable font and foreground style. Closes sozercan#435
- Remove sourcePlaylistId assignment (belongs in sidebar-highlight PR, not this one). Fixes build failure: PlayerService has no sourcePlaylistId member on this branch. - Extract PlaylistTrackRow to its own file to bring PlaylistDetailView.swift under the 900-line SwiftLint limit (934 -> 790 lines). - Extract HoverUnderlineNavigationLink to its own file (was private, now internal for cross-file reuse). - Mark unused fallbackAuthor parameter with _ (SwiftFormat unusedArguments rule). - Use keyPath syntax for contains(where:) (SwiftFormat preferKeyPath rule).
|
@codex review |
Row Button won the hit test on the right portion of the link, so clicks past ~2/3 of the name played the track instead of navigating. Measured: link frame matched the text width exactly (72.5pt), so sizing was fine — the label's composite hit shape was the problem. contentShape now closes the label modifier chain, vertical padding widens the target band, and pointerStyle(.link) restores the missing hand cursor.
Album track entries carry no per-track artist objects, so trackArtists returned nil and every album row rendered plain text while the header link worked. Fall back to the detail author when it is navigable and its name matches the displayed subtitle.
|
Suggested follow-up, deliberately not in this PR. The hit-testing bug fixed here has a structural root cause: The fix in this PR tunes the label's hit shape. It works and it's verified, but it doesn't remove the ambiguity. A more robust structure inverts the layering so hit priority is z-order instead of arbitration: HStack { … artist links, LikeButton … }
.background(
Button(action: onPlay) { Color.clear.contentShape(.rect) }
.buttonStyle(.interactiveRow(cornerRadius: 6))
)Foreground controls always win where they are; the background catches everything else. Two reasons to do it separately rather than here: it's a bigger diff than this PR's reviewers are expecting, and |
loadMoreBlockedWhileRefreshRewindsHistoryCursor simulated an in-flight refresh with a 100ms getHistory delay, then polled every 25ms before calling loadMore. On loaded runners the poll returned after refresh had already completed, so the !isRefreshingHistory guard no longer blocked loadMore: 3 continuation calls instead of 1, "Older" appended. Replace the delay with an explicit continuation gate on the mock, matching the existing shouldWaitForRemoveSongFromPlaylistResponse pattern, so refresh cannot finish before loadMore is attempted.
CI: fixed the
|
1 participant
Description
Artist names in playlist track rows are currently plain
Text— clicking the row only plays the track. This PR makes artist names individually clickableNavigationLinks when the artist has a navigable ID, while keeping the row's play-on-click behavior intact. Non-navigable artists remain plain text.AI Prompt (Optional)
🤖 AI Prompt Used
AI Tool: Claude
Type of Change
Related Issues
Closes #435
Changes Made
PlaylistDetailView.swift:HoverUnderlineNavigationLinkwithfontandforegroundStyleparameters so it can match the track row subtitle styling (12pt,.secondary)artists: [Artist]?property toPlaylistTrackRowTextwithartistLinksViewwhen navigable artists are available; falls back to plainText(subtitle)otherwisehasNavigableArtistscomputed property andartistLinksViewthat renders each artist as aHoverUnderlineNavigationLink(or plainTextfor non-navigable artists), separated by ", "trackArtists(for:fallbackAuthor:)helper that returns the uniqueArtistarray for linkingtrackRowcall site to pass theartistsparametertrackArtists(for:fallbackAuthor:)now falls back to the detail author when a track carries no artist objects of its own — see "Album track rows" belowHoverUnderlineNavigationLink.swift:contentShape(.rect)now closes the label modifier chain, vertical padding widens the hit band, andpointerStyle(.link)provides the hand cursorPlaylistTrackRow.swift: trailingSpacer(minLength: 0)in the artistHStackso leftover row width is absorbed by the spacer rather than distributed to the linksScreencast
Screen.Recording.2026-08-15.at.23.29.51-compressed.mp4
Testing
swift test --skip KasetUITests) — 2956 tests in 232 suitesVerified in a packaged build: playlist rows and album rows, hover underline, click-through to the artist page, and click-elsewhere-in-row still plays the track.
Checklist
swiftlint --strict && swiftformat .No automated test was added: the defects here are SwiftUI hit-testing and pointer-region behavior, which have no test seam in this codebase —
NSHostingViewflattens the hierarchy, sohitTestcannot distinguish the link from the row button.Additional Notes
Hit-testing fix
The first version of this PR had a real defect: only part of each artist name was clickable. Clicks past roughly two-thirds of the name fell through to the row and played the track instead of navigating, and the link cursor never appeared at all.
Instrumenting the real view (geometry logged from a packaged build) ruled out the obvious explanation — the link frame already matched the rendered text exactly:
onHoveralso fired across the full width (trueat localX 2.46,falseat 72.39). So sizing and hover tracking were both correct; what differed was click hit-testing — the rowButtonwon on the right portion. Confirmed by tap probes: clicks at localX 27.0 and 48.7 activated the link, a click at ~68.5 triggered the row's play action.The fix makes
contentShape(.rect)the last modifier in the label chain (nothing after it to disturb the composite hit shape), adds a little vertical padding to widen the target band, and addspointerStyle(.link).This also corrects an assumption in the original version of this description, which claimed the nested
NavigationLink"intercepts taps on its own area". It does not do so reliably — that is the bug.Album track rows
Album track entries returned by YouTube carry no per-track artist objects, so
trackArtistsreturnedniland every album row rendered plain text even though the album header's artist link worked.trackArtistsnow falls back to the detail author, guarded twice: only when that author has a navigable ID, and only when its name matches the subtitle the row is actually displaying. Albums whose author is anArtist.inline(…)placeholder degrade to plain text exactly as before, rather than rendering a dead link.This is scope beyond the original "playlist track rows" framing, so calling it out explicitly.
Known follow-up (not in this PR)
PlaylistTrackRowwraps the whole row inButton(action: onPlay)and puts other interactive controls (artist links,LikeButton) inside that button's label. Nested controls in aButtonlabel make SwiftUI arbitrate who wins a click, and that arbitration is the root cause of the hit-testing bug above; the fix here tunes the label's hit shape rather than removing the ambiguity.A more robust structure inverts the layering so hit priority is z-order rather than arbitration:
Foreground controls always win where they are; the background catches everything else. Worth doing as a shared row container rather than inline, since
.interactiveRowis a shared button style and this row shape likely repeats in album/search/queue views — which may carry the same latent bug. Left out of this PR to keep the diff reviewable.