Skip to content

feat(player): add now playing lyrics view and polish synced lyric presentation#267

Open
yeonfish6040 wants to merge 15 commits into
sozercan:mainfrom
yeonfish6040:player-lyrics-ui-split
Open

feat(player): add now playing lyrics view and polish synced lyric presentation#267
yeonfish6040 wants to merge 15 commits into
sozercan:mainfrom
yeonfish6040:player-lyrics-ui-split

Conversation

@yeonfish6040

Copy link
Copy Markdown

Description

Adds the now playing lyrics surface and refines the synced lyric presentation, artwork layout, scrolling behavior, and thumbnail handling.

AI Prompt (Optional)

🤖 AI Prompt Used

N/A - Manual implementation

AI Tool: OpenAI Codex

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 📚 Documentation update
  • 🎨 UI/UX improvement
  • ♻️ Refactoring (no functional changes)
  • 🧪 Test update
  • 🔧 Build/CI configuration

Related Issues

Changes Made

  • Added the now playing lyrics view.
  • Refined synced lyrics wrapping, highlighting, and loading behavior.
  • Improved lyrics scrolling inside the now playing panel.
  • Adjusted lyrics panel sizing and inset handling for the window.
  • Reworked artwork alignment in the now playing layout.
  • Updated thumbnail quality handling.
  • Refined lyrics color presentation.
  • Added and updated tests around lyric display behavior.

Testing

  • Unit tests pass (xcodebuild test -only-testing:KasetTests)
  • Manual testing performed
  • UI tested on macOS 26+

Checklist

  • My code follows the project's style guidelines
  • I have run swiftlint --strict && swiftformat .
  • I have added tests that prove my fix/feature works
  • New and existing unit tests pass locally
  • I have updated documentation if needed
  • I have checked for any performance implications
  • My changes generate no new warnings

Screenshots

Additional Notes

Copilot AI review requested due to automatic review settings May 28, 2026 06:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new full-screen "Now Playing" lyrics surface with album art and transport controls, refactors SyncedLyricsDisplayView/SyncedLineView to be reusable and visually richer, and fixes related caching/race issues in CachedAsyncImage and ImageCache.

Changes:

  • New NowPlayingLyricsView presented as an overlay from MainWindow, triggered by tapping the PlayerBar, with a new showNowPlayingLyrics state on PlayerService.
  • Refactor of SyncedLyricsDisplayView to support custom scroll anchor / insets / disabled auto-scroll, and visual polish (color helpers, scale-based sizing, pre-wrapped text) in SyncedLineView.
  • Image caching improvements: CachedAsyncImage no longer flashes during URL changes, and ImageCache keys memory entries by (url, targetSize).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Sources/Kaset/Views/NowPlayingLyricsView.swift New full-screen now-playing surface with artwork, controls, and lyrics.
Sources/Kaset/Views/SyncedLyricsDisplayView.swift Parameterized scrolling/insets and reworked line styling.
Sources/Kaset/Views/MainWindow.swift Overlays NowPlayingLyricsView and disables hit testing on background.
Sources/Kaset/Views/PlayerBar.swift Tap on player bar opens the now-playing view.
Sources/Kaset/Views/LyricsView.swift Shows small inline spinner when refreshing already-available lyrics.
Sources/Kaset/Views/CachedAsyncImage.swift Prevents flashing/race on URL changes via loadingURL guard.
Sources/Kaset/Utilities/ImageCache.swift Memory cache keyed by url + target size.
Sources/Kaset/Services/Player/PlayerService.swift Adds showNowPlayingLyrics state.
Sources/Kaset/Services/AI/FoundationModelsService.swift Drops await on onPartial callback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Kaset/Views/NowPlayingLyricsView.swift Outdated
.font(.system(size: 34, weight: .semibold))
Text("No Lyrics Available")
.font(.system(size: 22, weight: .bold))
}
self.lockedArtworkVideoId = track.videoId
}

guard let artworkURL = track.thumbnailURL?.highQualityThumbnailURL ?? track.thumbnailURL else { return }
Comment on lines +215 to 225
private func cacheKey(for url: URL, targetSize: CGSize? = nil) -> String {
let targetComponent = if let targetSize {
"\(Int(targetSize.width.rounded()))x\(Int(targetSize.height.rounded()))"
} else {
"original"
}

let data = Data("\(url.absoluteString)|\(targetComponent)".utf8)
let hash = SHA256.hash(data: data)
return hash.compactMap { String(format: "%02x", $0) }.joined()
}
Comment on lines +168 to +180
.contentShape(Rectangle())
.onTapGesture {
guard self.playerService.currentTrack != nil,
!self.showsSeekControls
else {
return
}

HapticService.navigation()
withAnimation(AppAnimation.standard) {
self.playerService.showNowPlayingLyrics = true
}
}
self.updateLyricsPolling(for: self.syncedLyricsService.currentLyrics)
}
.onDisappear {
SingletonPlayerWebView.shared.stopLyricsPoll()
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 28, 2026 08:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Comment on lines +76 to +78
.onAppear {
self.updateLyricsPolling(for: self.syncedLyricsService.currentLyrics)
}
Comment on lines +355 to +368
Slider(value: self.$volumeValue, in: 0 ... 1) { editing in
if editing {
self.isAdjustingVolume = true
} else {
self.isAdjustingVolume = false
Task { await self.playerService.setVolume(self.volumeValue) }
}
}
.controlSize(.small)
.tint(.white)
.onChange(of: self.volumeValue) { _, newValue in
guard self.isAdjustingVolume else { return }
Task { await self.playerService.setVolume(newValue) }
}
Comment on lines +142 to +148
self.lyricsPane(availableSize: availableSize)
.padding(.horizontal, 14)
.frame(
maxWidth: metrics.lyricsPaneWidth,
maxHeight: max(0, availableSize.height - 112),
alignment: .top
)
Comment on lines +53 to +54
self.onFailure?()
}
Comment on lines +215 to 225
private func cacheKey(for url: URL, targetSize: CGSize? = nil) -> String {
let targetComponent = if let targetSize {
"\(Int(targetSize.width.rounded()))x\(Int(targetSize.height.rounded()))"
} else {
"original"
}

let data = Data("\(url.absoluteString)|\(targetComponent)".utf8)
let hash = SHA256.hash(data: data)
return hash.compactMap { String(format: "%02x", $0) }.joined()
}
for try await snapshot in stream {
partial = snapshot.content
await onPartial(snapshot.content)
onPartial(snapshot.content)
@yeonfish6040

Copy link
Copy Markdown
Author
image Nice

Copilot AI review requested due to automatic review settings May 30, 2026 00:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Comment on lines +215 to +222
private func cacheKey(for url: URL, targetSize: CGSize? = nil) -> String {
let targetComponent = if let targetSize {
"\(Int(targetSize.width.rounded()))x\(Int(targetSize.height.rounded()))"
} else {
"original"
}

let data = Data("\(url.absoluteString)|\(targetComponent)".utf8)
Comment on lines 244 to 247
for try await snapshot in stream {
partial = snapshot.content
await onPartial(snapshot.content)
onPartial(snapshot.content)
}
Comment on lines +580 to +588
private struct LayoutMetrics {
let topPadding: CGFloat
let horizontalPadding: CGFloat
let bottomPadding: CGFloat
let columnSpacing: CGFloat
let artworkSize: CGFloat
let artworkColumnWidth: CGFloat
let lyricsPaneWidth: CGFloat
}
Comment on lines +168 to +180
.contentShape(Rectangle())
.onTapGesture {
guard self.playerService.currentTrack != nil,
!self.showsSeekControls
else {
return
}

HapticService.navigation()
withAnimation(AppAnimation.standard) {
self.playerService.showNowPlayingLyrics = true
}
}
Comment on lines +36 to 42
.onChange(of: self.url) { _, newURL in
if newURL == nil {
self.loadingURL = nil
self.image = nil
self.isLoaded = false
}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants