address feedback: optimistic star and menu polish

- star/unstar updates the local row immediately and rolls back with a toast if the server
  rejects it, instead of waiting on the response
- the menu collapses with an animation on dismiss
- star/unstar keeps the menu open and toggles its item in place
- the album/artist rows show the target's name instead of "View album"/"View artist"
- the menu header shows the cover art thumbnail

app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt

@@ -18,6 +18,7 @@ import com.subtracks.data.prefs.SongSort
 import com.subtracks.data.prefs.StarredFilter
 import com.subtracks.data.source.StarType
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.first
 
 class LibraryRepository(
     private val db: SubtracksDatabase,
@@ -249,11 +250,26 @@ class LibraryRepository(
         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 runCatching { source.setStar(type, id, starred) }
-            .onSuccess { updateStarred(sourceId, type, id, if (starred) System.currentTimeMillis() else null) }
-            .onFailure { showMessage("Could not update star") }
+            .onFailure {
+                updateStarred(sourceId, type, id, previous)
+                showMessage("Could not update star")
+            }
     }
 
+    private suspend fun starredValue(
+        sourceId: Long,
+        type: StarType,
+        id: String,
+    ): Long? =
+        when (type) {
+            StarType.Song -> song(sourceId, id).first()?.starred
+            StarType.Album -> album(sourceId, id).first()?.starred
+            StarType.Artist -> artist(sourceId, id).first()?.starred
+        }
+
     private suspend fun updateStarred(
         sourceId: Long,
         type: StarType,

app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt

@@ -1,6 +1,9 @@
 package com.subtracks.ui.components
 
 import androidx.compose.foundation.clickable
+import androidx.compose.foundation.layout.size
+import androidx.compose.foundation.shape.CircleShape
+import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.rounded.Album
 import androidx.compose.material.icons.rounded.Person
@@ -15,12 +18,22 @@ import androidx.compose.material3.ListItemDefaults
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.ModalBottomSheet
 import androidx.compose.material3.Text
+import androidx.compose.material3.rememberModalBottomSheetState
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.clip
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.vector.ImageVector
 import androidx.compose.ui.text.style.TextOverflow
+import androidx.compose.ui.unit.dp
+import com.subtracks.data.model.CoverArtRef
 import com.subtracks.data.source.StarType
+import kotlinx.coroutines.launch
 import com.subtracks.data.model.Album as AlbumModel
 import com.subtracks.data.model.Artist as ArtistModel
 import com.subtracks.data.model.Playlist as PlaylistModel
@@ -29,9 +42,11 @@ import com.subtracks.data.model.Song as SongModel
 sealed interface MenuTarget {
     val title: String
     val subtitle: String?
+    val coverArt: CoverArtRef?
 
     data class Song(
         val song: SongModel,
+        override val coverArt: CoverArtRef? = null,
     ) : MenuTarget {
         override val title: String get() = song.title
 
@@ -40,6 +55,7 @@ sealed interface MenuTarget {
 
     data class Album(
         val album: AlbumModel,
+        override val coverArt: CoverArtRef? = null,
     ) : MenuTarget {
         override val title: String get() = album.name
 
@@ -48,6 +64,7 @@ sealed interface MenuTarget {
 
     data class Artist(
         val artist: ArtistModel,
+        override val coverArt: CoverArtRef? = null,
     ) : MenuTarget {
         override val title: String get() = artist.name
 
@@ -56,6 +73,7 @@ sealed interface MenuTarget {
 
     data class Playlist(
         val playlist: PlaylistModel,
+        override val coverArt: CoverArtRef? = null,
     ) : MenuTarget {
         override val title: String get() = playlist.name
 
@@ -81,54 +99,64 @@ fun ItemContextMenu(
     actions: ItemActions,
     onDismiss: () -> Unit,
 ) {
-    ModalBottomSheet(onDismissRequest = onDismiss) {
-        val dismissThen: (() -> Unit) -> Unit = { action ->
+    val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)
+    val scope = rememberCoroutineScope()
+
+    fun dismiss(after: () -> Unit = {}) {
+        scope.launch {
+            sheetState.hide()
+            after()
             onDismiss()
-            action()
         }
+    }
+
+    ModalBottomSheet(
+        onDismissRequest = { dismiss() },
+        sheetState = sheetState,
+    ) {
         MenuHeader(target)
         when (target) {
             is MenuTarget.Song -> {
                 actions.playSong?.let { play ->
-                    MenuItem(Icons.Rounded.PlayArrow, "Play") { dismissThen { play(target.song) } }
-                }
-                MenuItem(starIcon(target.song.starred), starLabel(target.song.starred)) {
-                    dismissThen { actions.setStar(StarType.Song, target.song.id, target.song.starred == null) }
+                    MenuItem(Icons.Rounded.PlayArrow, "Play") { dismiss { play(target.song) } }
                 }
+                StarItem(target.song.starred) { starring -> actions.setStar(StarType.Song, target.song.id, starring) }
                 target.song.albumId?.let { albumId ->
                     actions.viewAlbum?.let { view ->
-                        MenuItem(Icons.Rounded.Album, "View album") { dismissThen { view(albumId) } }
+                        MenuItem(Icons.Rounded.Album, target.song.album?.takeIf { it.isNotBlank() } ?: "Album") {
+                            dismiss { view(albumId) }
+                        }
                     }
                 }
                 target.song.artistId?.let { artistId ->
                     actions.viewArtist?.let { view ->
-                        MenuItem(Icons.Rounded.Person, "View artist") { dismissThen { view(artistId) } }
+                        MenuItem(Icons.Rounded.Person, target.song.artist?.takeIf { it.isNotBlank() } ?: "Artist") {
+                            dismiss { view(artistId) }
+                        }
                     }
                 }
             }
 
             is MenuTarget.Album -> {
-                MenuItem(Icons.Rounded.PlayArrow, "Play") { dismissThen { actions.playAlbum(target.album) } }
-                MenuItem(Icons.Rounded.Shuffle, "Shuffle") { dismissThen { actions.shuffleAlbum(target.album) } }
-                MenuItem(starIcon(target.album.starred), starLabel(target.album.starred)) {
-                    dismissThen { actions.setStar(StarType.Album, target.album.id, target.album.starred == null) }
-                }
+                MenuItem(Icons.Rounded.PlayArrow, "Play") { dismiss { actions.playAlbum(target.album) } }
+                MenuItem(Icons.Rounded.Shuffle, "Shuffle") { dismiss { actions.shuffleAlbum(target.album) } }
+                StarItem(target.album.starred) { starring -> actions.setStar(StarType.Album, target.album.id, starring) }
                 target.album.artistId?.let { artistId ->
                     actions.viewArtist?.let { view ->
-                        MenuItem(Icons.Rounded.Person, "View artist") { dismissThen { view(artistId) } }
+                        MenuItem(Icons.Rounded.Person, target.album.albumArtist?.takeIf { it.isNotBlank() } ?: "Artist") {
+                            dismiss { view(artistId) }
+                        }
                     }
                 }
             }
 
             is MenuTarget.Artist -> {
-                MenuItem(starIcon(target.artist.starred), starLabel(target.artist.starred)) {
-                    dismissThen { actions.setStar(StarType.Artist, target.artist.id, target.artist.starred == null) }
-                }
+                StarItem(target.artist.starred) { starring -> actions.setStar(StarType.Artist, target.artist.id, starring) }
             }
 
             is MenuTarget.Playlist -> {
-                MenuItem(Icons.Rounded.PlayArrow, "Play") { dismissThen { actions.playPlaylist(target.playlist) } }
-                MenuItem(Icons.Rounded.Shuffle, "Shuffle") { dismissThen { actions.shufflePlaylist(target.playlist) } }
+                MenuItem(Icons.Rounded.PlayArrow, "Play") { dismiss { actions.playPlaylist(target.playlist) } }
+                MenuItem(Icons.Rounded.Shuffle, "Shuffle") { dismiss { actions.shufflePlaylist(target.playlist) } }
             }
         }
     }
@@ -155,10 +183,30 @@ private fun MenuHeader(target: MenuTarget) {
                     )
                 }
             },
+        leadingContent = {
+            CoverArt(
+                ref = target.coverArt,
+                name = target.title,
+                modifier = Modifier.size(48.dp).clip(if (target is MenuTarget.Artist) CircleShape else RoundedCornerShape(8.dp)),
+            )
+        },
         colors = ListItemDefaults.colors(containerColor = Color.Transparent),
     )
 }
 
+@Composable
+private fun StarItem(
+    current: Long?,
+    onToggle: (Boolean) -> Unit,
+) {
+    var starred by remember(current) { mutableStateOf(current) }
+    MenuItem(starIcon(starred), starLabel(starred)) {
+        val starring = starred == null
+        starred = if (starring) System.currentTimeMillis() else null
+        onToggle(starring)
+    }
+}
+
 @Composable
 private fun MenuItem(
     icon: ImageVector,

app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt

@@ -77,7 +77,7 @@ fun AlbumDetailRoute(
         onSongLongClick = { menuTarget = it },
         onShuffle = viewModel::shuffle,
         onPlay = viewModel::playAll,
-        onMore = { album?.let { menuTarget = MenuTarget.Album(it) } },
+        onMore = { album?.let { menuTarget = MenuTarget.Album(it, viewModel.coverArt(it.coverArt, true)) } },
         playingSongId = playback.item?.id.takeIf { context?.kind == QueueKind.Album && context.refId == albumId },
     )
     menuTarget?.let { target ->
@@ -161,7 +161,7 @@ fun AlbumDetailScreen(
                         modifier =
                             rowModifier.combinedClickable(
                                 onClick = { onSongClick(index) },
-                                onLongClick = { onSongLongClick(MenuTarget.Song(song)) },
+                                onLongClick = { onSongLongClick(MenuTarget.Song(song, coverArt(album?.coverArt, true))) },
                             ),
                     )
                 }

app/src/main/kotlin/com/subtracks/ui/library/AlbumsContent.kt

@@ -94,7 +94,7 @@ fun AlbumsContent(
                                     .clip(RoundedCornerShape(2.dp))
                                     .combinedClickable(
                                         onClick = { onAlbumClick(album) },
-                                        onLongClick = { onLongClick(MenuTarget.Album(album)) },
+                                        onLongClick = { onLongClick(MenuTarget.Album(album, art)) },
                                     ).testTag(ALBUM_COVER_TAG),
                         )
                     }

app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt

@@ -116,7 +116,7 @@ fun ArtistDetailRoute(
         onBack = onBack,
         onAlbumClick = onAlbumClick,
         onAlbumLongClick = { menuTarget = it },
-        onMore = { artist?.let { menuTarget = MenuTarget.Artist(it) } },
+        onMore = { artist?.let { menuTarget = MenuTarget.Artist(it, artThumbnail ?: art) } },
     )
     menuTarget?.let { target ->
         ItemContextMenu(target = target, actions = actions, onDismiss = { menuTarget = null })
@@ -238,7 +238,7 @@ fun ArtistDetailScreen(
                                         end = if (index % 2 == 1) 16.dp else 0.dp,
                                     ).combinedClickable(
                                         onClick = { onAlbumClick(album) },
-                                        onLongClick = { onAlbumLongClick(MenuTarget.Album(album)) },
+                                        onLongClick = { onAlbumLongClick(MenuTarget.Album(album, coverArt(album.coverArt, true))) },
                                     ),
                         ) {
                             CoverArt(

app/src/main/kotlin/com/subtracks/ui/library/ArtistsContent.kt

@@ -98,7 +98,7 @@ fun ArtistsContent(
                             modifier =
                                 Modifier.combinedClickable(
                                     onClick = { onArtistClick(artist) },
-                                    onLongClick = { onLongClick(MenuTarget.Artist(artist)) },
+                                    onLongClick = { onLongClick(MenuTarget.Artist(artist, coverArt(artist.coverArt, true))) },
                                 ),
                             colors = ListItemDefaults.colors(containerColor = Color.Transparent),
                         )

app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt

@@ -61,7 +61,7 @@ fun PlaylistDetailRoute(
         onSongLongClick = { menuTarget = it },
         onShuffle = viewModel::shuffle,
         onPlay = viewModel::playAll,
-        onMore = { playlist?.let { menuTarget = MenuTarget.Playlist(it) } },
+        onMore = { playlist?.let { menuTarget = MenuTarget.Playlist(it, viewModel.coverArt(it.coverArt, true)) } },
         playingSongId = playback.item?.id.takeIf { context?.kind == QueueKind.Playlist && context.refId == playlistId },
     )
     menuTarget?.let { target ->
@@ -116,7 +116,7 @@ fun PlaylistDetailScreen(
                         modifier =
                             rowModifier.combinedClickable(
                                 onClick = { onSongClick(index) },
-                                onLongClick = { onSongLongClick(MenuTarget.Song(item.song)) },
+                                onLongClick = { onSongLongClick(MenuTarget.Song(item.song, coverArt(item.coverArt, true))) },
                             ),
                     )
                 }

app/src/main/kotlin/com/subtracks/ui/library/PlaylistsContent.kt

@@ -96,7 +96,7 @@ fun PlaylistsContent(
                             modifier =
                                 Modifier.combinedClickable(
                                     onClick = { onPlaylistClick(playlist) },
-                                    onLongClick = { onLongClick(MenuTarget.Playlist(playlist)) },
+                                    onLongClick = { onLongClick(MenuTarget.Playlist(playlist, coverArt(playlist.coverArt, true))) },
                                 ),
                             colors = ListItemDefaults.colors(containerColor = Color.Transparent),
                         )

app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt

@@ -91,7 +91,7 @@ fun SongsContent(
                             modifier =
                                 Modifier.combinedClickable(
                                     onClick = { onSongClick(item.song.id) },
-                                    onLongClick = { onLongClick(MenuTarget.Song(item.song)) },
+                                    onLongClick = { onLongClick(MenuTarget.Song(item.song, coverArt(item.coverArt, true))) },
                                 ),
                         )
                     }

app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt

@@ -138,7 +138,7 @@ fun NowPlayingRoute(
         onPrevious = controller::previous,
         onShuffle = controller::toggleShuffle,
         onRepeat = controller::cycleRepeat,
-        onMore = { song?.let { menuTarget = MenuTarget.Song(it) } },
+        onMore = { song?.let { menuTarget = MenuTarget.Song(it, thumbnail) } },
         onSeek = controller::seekTo,
         modifier = modifier,
     )

app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt

@@ -9,8 +9,11 @@ import com.subtracks.data.db.SubtracksDatabase
 import com.subtracks.data.model.Song
 import com.subtracks.data.prefs.fakeUserPreferences
 import com.subtracks.data.source.StarType
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import okhttp3.OkHttpClient
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
@@ -22,6 +25,7 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import java.util.concurrent.TimeUnit
 
 @RunWith(AndroidJUnit4::class)
 class LibraryRepositoryTest {
@@ -80,7 +84,19 @@ class LibraryRepositoryTest {
         }
 
     @Test
-    fun aRejectedStarLeavesTheRowUntouchedAndReports() =
+    fun starringUpdatesTheRowBeforeTheServerResponds() =
+        runBlocking {
+            withSource { server ->
+                server.enqueue(ok().setBodyDelay(10, TimeUnit.SECONDS))
+
+                val call = launch { repository.setStar(StarType.Song, "s1", true) }
+                withTimeout(3_000) { while (starred() == null) delay(10) }
+                call.cancel()
+            }
+        }
+
+    @Test
+    fun aRejectedStarRollsBackAndReports() =
         runBlocking {
             withSource { server ->
                 server.enqueue(failed(40))
@@ -92,6 +108,22 @@ class LibraryRepositoryTest {
             }
         }
 
+    @Test
+    fun aRejectedUnstarRestoresTheStarredRow() =
+        runBlocking {
+            withSource { server ->
+                server.enqueue(ok())
+                repository.setStar(StarType.Song, "s1", true)
+                val starredAt = starred()
+                server.enqueue(failed(40))
+
+                assertTrue(repository.setStar(StarType.Song, "s1", false).isFailure)
+
+                assertEquals(starredAt, starred())
+                assertEquals(listOf("Could not update star"), messages)
+            }
+        }
+
     private suspend fun withSource(block: suspend (MockWebServer) -> Unit) {
         val server = MockWebServer()
         server.start()