From 51ed8fdd6b34cb3737bc222b7992582b0ae7febf Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:24:49 +0000 Subject: [PATCH] address review: pre-P shadow fallback and safer notification star - draw the play halo as an offset copy below API 28, where path shadow layers are ignored, and skip the inner shadow there too - add a repository toggleStar that reads and writes under the star lock, so fast double taps on the notification cannot desync - reset the star button when a track starts, so a failed source lookup cannot leave the previous song's state showing --- .../subtracks/data/repo/LibraryRepository.kt | 55 ++++++++++++------- .../com/subtracks/playback/PlaybackService.kt | 5 +- .../com/subtracks/ui/library/SongsContent.kt | 25 +++++++-- 3 files changed, 60 insertions(+), 25 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index aaee318a..8b85a653 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -209,30 +209,47 @@ class LibraryRepository( starScope.launch { setStar(type, id, starred) } } + fun toggleStar( + type: StarType, + id: String, + ) { + starScope.launch { + starLock.withLock { + val sourceId = sourceRepository.activeSourceIdOnce() ?: return@withLock + setStarLocked(type, id, starredValue(sourceId, type, id) == null) + } + } + } + suspend fun setStar( type: StarType, id: String, starred: Boolean, - ): Result = - starLock.withLock { - val sourceId = sourceRepository.activeSourceIdOnce() - val source = sourceRepository.activeMusicSource() - if (sourceId == null || source == null) { - return@withLock Result.failure(IllegalStateException("No active server")) - } - val previous = starredValue(sourceId, type, id) - updateStarred(sourceId, type, id, if (starred) System.currentTimeMillis() else null) - try { - source.setStar(type, id, starred) - Result.success(Unit) - } catch (cancellation: CancellationException) { - throw cancellation - } catch (failure: Exception) { - withContext(NonCancellable) { updateStarred(sourceId, type, id, previous) } - showMessage("Could not update star") - Result.failure(failure) - } + ): Result = starLock.withLock { setStarLocked(type, id, starred) } + + private suspend fun setStarLocked( + type: StarType, + id: String, + starred: Boolean, + ): Result { + val sourceId = sourceRepository.activeSourceIdOnce() + val source = sourceRepository.activeMusicSource() + if (sourceId == null || source == null) { + return Result.failure(IllegalStateException("No active server")) } + val previous = starredValue(sourceId, type, id) + updateStarred(sourceId, type, id, if (starred) System.currentTimeMillis() else null) + return try { + source.setStar(type, id, starred) + Result.success(Unit) + } catch (cancellation: CancellationException) { + throw cancellation + } catch (failure: Exception) { + withContext(NonCancellable) { updateStarred(sourceId, type, id, previous) } + showMessage("Could not update star") + Result.failure(failure) + } + } private suspend fun starredValue( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt index d2dd17ff..0b1ec74d 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt @@ -147,12 +147,13 @@ class PlaybackService : MediaSessionService() { private fun observeStar(songId: String?) { starJob?.cancel() currentSongId = songId + starred = false val library = library ?: return if (songId == null) { - starred = false session?.setMediaButtonPreferences(emptyList()) return } + session?.setMediaButtonPreferences(listOf(starButton(false))) starJob = scope.launch { val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch @@ -167,7 +168,7 @@ class PlaybackService : MediaSessionService() { private fun toggleStar() { val songId = currentSongId ?: return val library = library ?: return - library.star(StarType.Song, songId, !starred) + library.toggleStar(StarType.Song, songId) } private fun starButton(isStarred: Boolean): CommandButton = diff --git a/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt b/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt index 588f9376..43f0234c 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt @@ -1,5 +1,6 @@ package com.subtracks.ui.library +import android.os.Build import androidx.compose.foundation.Canvas import androidx.compose.foundation.border import androidx.compose.foundation.layout.Arrangement @@ -174,7 +175,9 @@ private fun CoverArtCell( colorFilter = if (isPlaying) DIM else null, ) if (isPlaying) { - Box(Modifier.matchParentSize().drawBehind { drawInnerShadow() }) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + Box(Modifier.matchParentSize().drawBehind { drawInnerShadow() }) + } Box( Modifier .matchParentSize() @@ -227,13 +230,27 @@ private fun PlayIndicator( val scale = minOf(size.width / PLAY_ARROW.viewportWidth, size.height / PLAY_ARROW.viewportHeight) withTransform({ scale(scale, scale, pivot = Offset.Zero) }) { drawIntoCanvas { canvas -> - val paint = + val path = PLAY_ARROW_PATH.asAndroidPath() + val glyph = android.graphics.Paint().apply { isAntiAlias = true color = tint.toArgb() - setShadowLayer(radius, 0f, 0f, PLAY_SHADOW.toArgb()) } - canvas.nativeCanvas.drawPath(PLAY_ARROW_PATH.asAndroidPath(), paint) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + glyph.setShadowLayer(radius, 0f, 0f, PLAY_SHADOW.toArgb()) + canvas.nativeCanvas.drawPath(path, glyph) + } else { + val halo = + android.graphics.Paint().apply { + isAntiAlias = true + color = PLAY_SHADOW.toArgb() + } + canvas.nativeCanvas.save() + canvas.nativeCanvas.translate(1f, 1.5f) + canvas.nativeCanvas.drawPath(path, halo) + canvas.nativeCanvas.restore() + canvas.nativeCanvas.drawPath(path, glyph) + } } } }