From 3eada70aac62971b454344be68b34c2efa285abe Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 16 Aug 2026 15:59:37 -0400 Subject: [PATCH] =?UTF-8?q?feat(android):=20Genres=20and=20Years=20browse?= =?UTF-8?q?=20axes=20in=20the=20Library=20=E2=80=94=20#2467?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #367 shipped genre and year browsing on web only, which left the web tab bar's own comment -- "mirrors Android's LibraryScreen" -- half aspirational. Android now has both, straight after Albums, in the same order the web bar uses. Server-backed, and that is the one real decision here. Every other Library tab reads Room, and building these indexes locally was the obvious move: the cache is a full mirror and carries both genre and releaseDate. It does not work. /api/library/sync hydrates through GetTracksByIDs, which has no missing_since filter, and neither SyncTrackWire nor CachedTrackEntity has a field for it -- so the cache holds tracks whose files are gone and cannot tell you which, while the browse index excludes them. A locally-derived index would quietly disagree with the server's and with the web client, and could offer a genre that exists only in missing files. Filed as #2704; until it is resolved these two tabs need a connection, and their empty states say what they are rather than looking broken. Genre is a query parameter end to end, never a path segment: "Rock/Pop" is a real ID3 tag and a slash does not survive a path. That is also why the drill-down is a second state inside the tab instead of a nav destination -- a route would have had to carry the label. Index shapes mirror web because the reasoning was already worked out there: genres default to count order, since raw tags carry a long tail of one-offs that A-Z buries the real genres under, with an A-Z chip for when you already know the name; years group by decade, newest first, because a flat list of every year in a decades-deep library is a wall of numbers. Page size matches web's BROWSE_PAGE_SIZE so "Load more (N left)" steps identically on both. The orderings and grouping are pure functions, tested: server order left alone under count sort, case-insensitive A-Z, no mutation of the loaded state's list, a slashed tag surviving the filter intact, decade bucketing including the boundary year, and a count label that stays blank rather than flashing "0 albums" while the first page loads. --- .../minstrel/api/endpoints/LibraryApi.kt | 46 +++ .../library/data/LibraryRepository.kt | 45 +++ .../minstrel/library/ui/BrowseViewModel.kt | 266 ++++++++++++++ .../minstrel/library/ui/GenresTab.kt | 341 ++++++++++++++++++ .../minstrel/library/ui/LibraryScreen.kt | 26 +- .../minstrel/library/ui/YearsTab.kt | 161 +++++++++ .../minstrel/models/wire/BrowseWire.kt | 33 ++ .../minstrel/library/ui/BrowseIndexTest.kt | 148 ++++++++ 8 files changed, 1060 insertions(+), 6 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/library/ui/BrowseViewModel.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/library/ui/GenresTab.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/library/ui/YearsTab.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/models/wire/BrowseWire.kt create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/library/ui/BrowseIndexTest.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/LibraryApi.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/LibraryApi.kt index 557f5443..51156f25 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/LibraryApi.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/LibraryApi.kt @@ -3,7 +3,10 @@ package com.fabledsword.minstrel.api.endpoints import com.fabledsword.minstrel.models.wire.AlbumDetailWire import com.fabledsword.minstrel.models.wire.ArtistDetailWire import com.fabledsword.minstrel.models.wire.ArtistWire +import com.fabledsword.minstrel.models.wire.GenreCountWire +import com.fabledsword.minstrel.models.wire.PagedAlbumsWire import com.fabledsword.minstrel.models.wire.TrackWire +import com.fabledsword.minstrel.models.wire.YearCountWire import retrofit2.http.GET import retrofit2.http.Path import retrofit2.http.Query @@ -54,6 +57,49 @@ interface LibraryApi { @GET("api/library/shuffle") suspend fun shuffleLibrary(@Query("limit") limit: Int = 100): List + // Browse axes (#367). Both indexes are unpaged by design: the client needs + // the whole set to render a browsable picker, and even a messy library + // yields hundreds of rows, not thousands. + // + // These read the server rather than the local cache on purpose. The cache + // is a full mirror of the library, but /api/library/sync ships tracks whose + // files are missing and carries no flag for it (#2704), while the browse + // index filters them out -- so a locally-computed index would disagree with + // the server's and with the web client. One source of truth wins over + // offline capability here until #2704 is resolved. + @GET("api/library/genres") + suspend fun getGenres(): List + + @GET("api/library/years") + suspend fun getAlbumYears(): List + + /** + * Albums carrying [genre] on any of their tracks. + * + * @Query, never @Path: "Rock/Pop" is a real ID3 tag and a slash cannot + * survive a path segment. Retrofit percent-encodes query values correctly; + * a @Path would either 404 or silently address a different genre. + */ + @GET("api/library/albums") + suspend fun getAlbumsByGenre( + @Query("genre") genre: String, + @Query("limit") limit: Int, + @Query("offset") offset: Int, + ): PagedAlbumsWire + + /** + * Albums released in an inclusive year range. Pass the same year twice for + * a single year. Sending a genre alongside these is a deliberate 400 on the + * server (`unsupported_filter_combination`) -- they are separate axes. + */ + @GET("api/library/albums") + suspend fun getAlbumsByYear( + @Query("year_from") yearFrom: Int, + @Query("year_to") yearTo: Int, + @Query("limit") limit: Int, + @Query("offset") offset: Int, + ): PagedAlbumsWire + private companion object { const val SIMILAR_ARTISTS_LIMIT = 12 const val TOP_TRACKS_LIMIT = 5 diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/data/LibraryRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/data/LibraryRepository.kt index 5dc4bc69..90749260 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/library/data/LibraryRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/data/LibraryRepository.kt @@ -161,7 +161,52 @@ class LibraryRepository @Inject constructor( suspend fun shuffleLibrary(limit: Int = SHUFFLE_DEFAULT_LIMIT): List = api.shuffleLibrary(limit = limit).map { it.toDomain() } + // ---- Browse axes (#367 / #2467) ---- + // + // Server-backed rather than cache-first, unlike everything above. The + // cache mirrors the whole library but includes tracks whose files are + // missing, with no flag to spot them (#2704), while the server's index + // excludes them -- so a locally-derived index would quietly disagree with + // the web client's. Revisit when #2704 lands. + + /** Genre index, ordered by track count then name (server order). */ + suspend fun genres(): List = + api.getGenres().map { GenreCount(genre = it.genre, trackCount = it.trackCount) } + + /** Year index, newest first. Albums with no release date are absent. */ + suspend fun albumYears(): List = + api.getAlbumYears().map { YearCount(year = it.year, albumCount = it.albumCount) } + + /** One page of albums carrying [genre] on any track. */ + suspend fun albumsByGenre(genre: String, limit: Int, offset: Int): AlbumPage { + val page = api.getAlbumsByGenre(genre = genre, limit = limit, offset = offset) + return AlbumPage(items = page.items.map { it.toDomain() }, total = page.total) + } + + /** One page of albums released in [year]. */ + suspend fun albumsByYear(year: Int, limit: Int, offset: Int): AlbumPage { + val page = api.getAlbumsByYear( + yearFrom = year, + yearTo = year, + limit = limit, + offset = offset, + ) + return AlbumPage(items = page.items.map { it.toDomain() }, total = page.total) + } + private companion object { const val SHUFFLE_DEFAULT_LIMIT = 100 } } + +/** One row of the genre index. */ +data class GenreCount(val genre: String, val trackCount: Int) + +/** One row of the year index. */ +data class YearCount(val year: Int, val albumCount: Int) + +/** + * A page of albums plus the server's total for the whole filter, which is + * what lets the UI say how many are left rather than just offering "more". + */ +data class AlbumPage(val items: List, val total: Int) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/BrowseViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/BrowseViewModel.kt new file mode 100644 index 00000000..e0229dc3 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/BrowseViewModel.kt @@ -0,0 +1,266 @@ +package com.fabledsword.minstrel.library.ui + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.fabledsword.minstrel.api.ErrorCopy +import com.fabledsword.minstrel.library.data.AlbumPage +import com.fabledsword.minstrel.library.data.GenreCount +import com.fabledsword.minstrel.library.data.LibraryRepository +import com.fabledsword.minstrel.library.data.YearCount +import com.fabledsword.minstrel.models.AlbumRef +import com.fabledsword.minstrel.shared.UiState +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import javax.inject.Inject + +/** How the genre index is ordered. */ +enum class GenreSort { + /** Server order: track count descending, name breaking ties. */ + COUNT, + + /** Alphabetical, case-insensitive. */ + NAME, +} + +/** + * Albums for whichever genre or year is currently drilled into. + * + * [total] is the server's count for the whole filter, not the loaded slice, + * so the UI can say how many are left instead of only offering "more". + */ +data class AlbumBrowseState( + val albums: List = emptyList(), + val total: Int = 0, + val loading: Boolean = false, + val failed: Boolean = false, +) { + val hasMore: Boolean get() = albums.size < total + val remaining: Int get() = (total - albums.size).coerceAtLeast(0) +} + +/** + * Backs the Genres and Years tabs (#2467), mirroring the web surfaces #367 + * shipped. + * + * Both indexes come from the server, which is a deliberate departure from the + * cache-first Artists/Albums tabs beside them: the local cache includes tracks + * whose files are missing and cannot tell you which (#2704), while the server's + * index excludes them, so a locally-derived index would disagree with the web + * client's. These two tabs therefore need a connection; the empty states say so + * rather than looking broken. + */ +// Two browse axes, each with an index, a filter/sort or grouping, a +// drill-down and a pager. The function count is two axes' worth of a +// cohesive surface; splitting into GenresViewModel + YearsViewModel would +// duplicate the shared paging body for no gain. +@Suppress("TooManyFunctions") +@HiltViewModel +class BrowseViewModel @Inject constructor( + private val repository: LibraryRepository, +) : ViewModel() { + + private val genresInternal = MutableStateFlow>>(UiState.Loading) + val genres: StateFlow>> = genresInternal.asStateFlow() + + private val yearsInternal = MutableStateFlow>>(UiState.Loading) + val years: StateFlow>> = yearsInternal.asStateFlow() + + private val genreFilterInternal = MutableStateFlow("") + val genreFilter: StateFlow = genreFilterInternal.asStateFlow() + + private val genreSortInternal = MutableStateFlow(GenreSort.COUNT) + val genreSort: StateFlow = genreSortInternal.asStateFlow() + + private val selectedGenreInternal = MutableStateFlow(null) + val selectedGenre: StateFlow = selectedGenreInternal.asStateFlow() + + private val selectedYearInternal = MutableStateFlow(null) + val selectedYear: StateFlow = selectedYearInternal.asStateFlow() + + private val genreAlbumsInternal = MutableStateFlow(AlbumBrowseState()) + val genreAlbums: StateFlow = genreAlbumsInternal.asStateFlow() + + private val yearAlbumsInternal = MutableStateFlow(AlbumBrowseState()) + val yearAlbums: StateFlow = yearAlbumsInternal.asStateFlow() + + // Guards against a slow response for a previously-selected genre/year + // landing after the user has moved on and painting over the new list. + // One counter per axis, since the two drill-downs are independent. + private var genreRequestToken = 0 + private var yearRequestToken = 0 + + init { + loadGenres() + loadYears() + } + + fun loadGenres() { + viewModelScope.launch { + genresInternal.value = UiState.Loading + genresInternal.value = runCatching { repository.genres() }.fold( + onSuccess = { if (it.isEmpty()) UiState.Empty else UiState.Success(it) }, + onFailure = { UiState.Error(ErrorCopy.fromThrowable(it)) }, + ) + } + } + + fun loadYears() { + viewModelScope.launch { + yearsInternal.value = UiState.Loading + yearsInternal.value = runCatching { repository.albumYears() }.fold( + onSuccess = { if (it.isEmpty()) UiState.Empty else UiState.Success(it) }, + onFailure = { UiState.Error(ErrorCopy.fromThrowable(it)) }, + ) + } + } + + fun setGenreFilter(value: String) { genreFilterInternal.value = value } + + fun setGenreSort(sort: GenreSort) { genreSortInternal.value = sort } + + /** Drill into [genre], or pass null to go back to the index. */ + fun selectGenre(genre: String?) { + selectedGenreInternal.value = genre + genreRequestToken += 1 + genreAlbumsInternal.value = AlbumBrowseState() + if (genre == null) return + fetchGenrePage(genre, offset = 0, token = genreRequestToken) + } + + fun loadMoreGenreAlbums() { + val genre = selectedGenreInternal.value ?: return + val state = genreAlbumsInternal.value + if (state.loading || !state.hasMore) return + fetchGenrePage(genre, offset = state.albums.size, token = genreRequestToken) + } + + fun retryGenreAlbums() { + selectedGenreInternal.value?.let { selectGenre(it) } + } + + /** Drill into [year], or pass null to go back to the index. */ + fun selectYear(year: Int?) { + selectedYearInternal.value = year + yearRequestToken += 1 + yearAlbumsInternal.value = AlbumBrowseState() + if (year == null) return + fetchYearPage(year, offset = 0, token = yearRequestToken) + } + + fun loadMoreYearAlbums() { + val year = selectedYearInternal.value ?: return + val state = yearAlbumsInternal.value + if (state.loading || !state.hasMore) return + fetchYearPage(year, offset = state.albums.size, token = yearRequestToken) + } + + fun retryYearAlbums() { + selectedYearInternal.value?.let { selectYear(it) } + } + + private fun fetchGenrePage(genre: String, offset: Int, token: Int) { + fetchPage( + state = genreAlbumsInternal, + offset = offset, + isCurrent = { token == genreRequestToken }, + fetch = { repository.albumsByGenre(genre, PAGE_SIZE, offset) }, + ) + } + + private fun fetchYearPage(year: Int, offset: Int, token: Int) { + fetchPage( + state = yearAlbumsInternal, + offset = offset, + isCurrent = { token == yearRequestToken }, + fetch = { repository.albumsByYear(year, PAGE_SIZE, offset) }, + ) + } + + /** + * The paging body both axes share: append on success, and drop the result + * entirely if the selection moved while the request was in flight. + */ + private fun fetchPage( + state: MutableStateFlow, + offset: Int, + isCurrent: () -> Boolean, + fetch: suspend () -> AlbumPage, + ) { + viewModelScope.launch { + state.value = state.value.copy(loading = true, failed = false) + runCatching { fetch() }.fold( + onSuccess = { page -> + if (!isCurrent()) return@launch + val merged = + if (offset == 0) page.items else state.value.albums + page.items + state.value = AlbumBrowseState( + albums = merged, + total = page.total, + loading = false, + failed = false, + ) + }, + onFailure = { + if (!isCurrent()) return@launch + state.value = state.value.copy(loading = false, failed = true) + }, + ) + } + } + + private companion object { + // Matches the web client's BROWSE_PAGE_SIZE so "Load more (N left)" + // steps at the same rate on both clients. + const val PAGE_SIZE = 50 + } +} + +/** + * Apply the current filter and sort to a genre index. + * + * Pure so the ordering rules are testable without a ViewModel. Sorting copies + * first: the input is the list held in the loaded state, and sorting in place + * would reorder what every other reader sees. + */ +fun visibleGenres( + genres: List, + filter: String, + sort: GenreSort, +): List { + val q = filter.trim() + val matched = + if (q.isEmpty()) genres else genres.filter { it.genre.contains(q, ignoreCase = true) } + return when (sort) { + // Server order is already count DESC then name; don't re-sort it. + GenreSort.COUNT -> matched + GenreSort.NAME -> matched.sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.genre }) + } +} + +/** A decade's worth of the year index, newest year first. */ +data class DecadeGroup( + val decade: Int, + val years: List, + val albumCount: Int, +) + +/** + * Group the year index by decade, newest first. + * + * A flat list of every year in a decades-deep library is a wall of numbers, and + * the decade is usually how someone actually thinks about it. Pure, for the + * same reason as [visibleGenres]. + */ +fun groupByDecade(years: List): List = + years.groupBy { (it.year / 10) * 10 } + .map { (decade, entries) -> + DecadeGroup( + decade = decade, + years = entries.sortedByDescending { it.year }, + albumCount = entries.sumOf { it.albumCount }, + ) + } + .sortedByDescending { it.decade } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/GenresTab.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/GenresTab.kt new file mode 100644 index 00000000..cf0290e2 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/GenresTab.kt @@ -0,0 +1,341 @@ +package com.fabledsword.minstrel.library.ui + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.grid.GridCells +import androidx.compose.foundation.lazy.grid.GridItemSpan +import androidx.compose.foundation.lazy.grid.LazyVerticalGrid +import androidx.compose.foundation.lazy.grid.items +import androidx.compose.material3.FilterChip +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.composables.icons.lucide.ArrowLeft +import com.composables.icons.lucide.Lucide +import com.composables.icons.lucide.LibraryBig +import com.fabledsword.minstrel.library.widgets.AlbumCard +import com.fabledsword.minstrel.models.AlbumRef +import com.fabledsword.minstrel.shared.UiState +import com.fabledsword.minstrel.shared.widgets.EmptyState +import com.fabledsword.minstrel.shared.widgets.ErrorRetry + +/** + * Genres tab (#2467) — the Android half of the browse axis #367 shipped on web. + * + * Two states in one tab rather than a navigation destination: the index, and + * the albums for a chosen genre. Back returns to the index. A route would have + * meant carrying the genre in the path, and "Rock/Pop" is a real ID3 tag whose + * slash a path segment cannot carry — the same reason the server takes it as a + * query parameter. + */ +@Composable +fun GenresTab( + onAlbumClick: (String) -> Unit, + viewModel: BrowseViewModel = hiltViewModel(), +) { + val selected by viewModel.selectedGenre.collectAsStateWithLifecycle() + val genre = selected + if (genre == null) { + GenreIndex(viewModel = viewModel) + } else { + GenreAlbums( + genre = genre, + viewModel = viewModel, + onAlbumClick = onAlbumClick, + ) + } +} + +@Composable +private fun GenreIndex(viewModel: BrowseViewModel) { + val state by viewModel.genres.collectAsStateWithLifecycle() + val filter by viewModel.genreFilter.collectAsStateWithLifecycle() + val sort by viewModel.genreSort.collectAsStateWithLifecycle() + + when (val s = state) { + UiState.Loading -> EmptyState( + title = "Reading your genres…", + body = "", + icon = Lucide.LibraryBig, + ) + UiState.Empty -> EmptyState( + title = "No genres found", + body = "Genres come from the genre tag on your audio files. If your " + + "library is tagged but this is empty, try a rescan from the admin " + + "screen.", + icon = Lucide.LibraryBig, + ) + is UiState.Error -> ErrorRetry( + message = s.message, + onRetry = viewModel::loadGenres, + ) + is UiState.Success -> { + val visible = visibleGenres(s.data, filter, sort) + Column(modifier = Modifier.fillMaxSize()) { + GenreIndexControls( + total = s.data.size, + shown = visible.size, + filter = filter, + sort = sort, + onFilterChange = viewModel::setGenreFilter, + onSortChange = viewModel::setGenreSort, + ) + if (visible.isEmpty()) { + EmptyState( + title = "No genres match \"${filter.trim()}\"", + body = "Try a shorter search.", + icon = Lucide.LibraryBig, + ) + } else { + LazyColumn(modifier = Modifier.fillMaxSize()) { + items(items = visible, key = { it.genre }) { row -> + GenreRow( + genre = row.genre, + trackCount = row.trackCount, + onClick = { viewModel.selectGenre(row.genre) }, + ) + HorizontalDivider() + } + } + } + } + } + } +} + +@Composable +private fun GenreIndexControls( + total: Int, + shown: Int, + filter: String, + sort: GenreSort, + onFilterChange: (String) -> Unit, + onSortChange: (GenreSort) -> Unit, +) { + Column(modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp)) { + OutlinedTextField( + value = filter, + onValueChange = onFilterChange, + label = { Text("Filter genres") }, + singleLine = true, + modifier = Modifier.fillMaxWidth(), + ) + Row( + modifier = Modifier.fillMaxWidth().padding(top = 8.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + // Count-first is the default because the head of that list is + // genuinely where you are going; raw tags carry a long tail of + // one-offs that A-Z would bury the real genres under. A-Z is here + // for when you already know roughly what it is called. + FilterChip( + selected = sort == GenreSort.COUNT, + onClick = { onSortChange(GenreSort.COUNT) }, + label = { Text("Most tracks") }, + ) + FilterChip( + selected = sort == GenreSort.NAME, + onClick = { onSortChange(GenreSort.NAME) }, + label = { Text("A–Z") }, + ) + Text( + text = if (filter.isBlank()) { + "$total genres" + } else { + "$shown of $total" + }, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } +} + +@Composable +private fun GenreRow(genre: String, trackCount: Int, onClick: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 14.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = genre, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), + ) + Text( + text = "$trackCount", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + +@Composable +private fun GenreAlbums( + genre: String, + viewModel: BrowseViewModel, + onAlbumClick: (String) -> Unit, +) { + val albums by viewModel.genreAlbums.collectAsStateWithLifecycle() + BrowseAlbumResults( + heading = genre, + subtitle = albumCountLabel(albums.total, albums.loading, albums.albums.size), + state = albums, + emptyTitle = "No albums for this genre", + emptyBody = "The library may have been rescanned since this list was built.", + onBack = { viewModel.selectGenre(null) }, + onRetry = viewModel::retryGenreAlbums, + onLoadMore = viewModel::loadMoreGenreAlbums, + onAlbumClick = onAlbumClick, + ) +} + +/** + * Shared results pane for both browse axes: a back affordance, a heading, the + * album grid, and the load-more footer. Genres and Years differ only in their + * heading and copy, so the layout lives once. + */ +@Composable +@Suppress("LongParameterList") // one presentational surface; all of it varies by axis +fun BrowseAlbumResults( + heading: String, + subtitle: String, + state: AlbumBrowseState, + emptyTitle: String, + emptyBody: String, + onBack: () -> Unit, + onRetry: () -> Unit, + onLoadMore: () -> Unit, + onAlbumClick: (String) -> Unit, +) { + Column(modifier = Modifier.fillMaxSize()) { + Row( + modifier = Modifier.fillMaxWidth().padding(start = 4.dp, end = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + IconButton(onClick = onBack) { + Icon(Lucide.ArrowLeft, contentDescription = "Back to the index") + } + Column(modifier = Modifier.weight(1f)) { + Text( + text = heading, + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + if (subtitle.isNotEmpty()) { + Text( + text = subtitle, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } + + when { + state.failed && state.albums.isEmpty() -> ErrorRetry( + message = "Couldn't load albums.", + onRetry = onRetry, + ) + state.loading && state.albums.isEmpty() -> EmptyState( + title = "Loading…", + body = "", + ) + state.albums.isEmpty() -> EmptyState(title = emptyTitle, body = emptyBody) + else -> BrowseAlbumGrid( + state = state, + onLoadMore = onLoadMore, + onAlbumClick = onAlbumClick, + ) + } + } +} + +@Composable +private fun BrowseAlbumGrid( + state: AlbumBrowseState, + onLoadMore: () -> Unit, + onAlbumClick: (String) -> Unit, +) { + LazyVerticalGrid( + // Same 176dp cell as the Albums tab, so a genre's grid and the full + // album grid line up rather than each inventing a column count. + columns = GridCells.Adaptive(minSize = 176.dp), + contentPadding = PaddingValues(8.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.fillMaxSize(), + ) { + items(items = state.albums, key = { it.id }) { album: AlbumRef -> + AlbumCard(album = album, onClick = { onAlbumClick(album.id) }) + } + item(span = { GridItemSpan(maxLineSpan) }) { + Box( + modifier = Modifier.fillMaxWidth().padding(vertical = 8.dp), + contentAlignment = Alignment.Center, + ) { + if (state.hasMore) { + // Explicit rather than infinite scroll, matching web: the + // remaining count is useful, and a browse axis is a place + // people skim rather than fall through. + TextButton(onClick = onLoadMore, enabled = !state.loading) { + Text( + if (state.loading) { + "Loading…" + } else { + "Load more (${state.remaining} left)" + }, + ) + } + } else { + Text( + text = "That's everything", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } + } +} + +/** + * "12 albums" once the total is known, and nothing at all while the first page + * is still in flight — a count that appears as 0 and then corrects itself reads + * as a bug. + */ +internal fun albumCountLabel(total: Int, loading: Boolean, loaded: Int): String = when { + loading && loaded == 0 -> "" + total == 1 -> "1 album" + else -> "$total albums" +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt index 1520d394..3e70fb43 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt @@ -52,8 +52,10 @@ import com.fabledsword.minstrel.shared.widgets.SkeletonAlbumTile import com.fabledsword.minstrel.shared.widgets.SkeletonArtistTile /** - * Library tab. Five-tab TabBar (Artists / Albums / History / Liked / - * Hidden) matching `flutter_client/lib/library/library_screen.dart`. + * Library tab. Seven-tab TabBar (Artists / Albums / Genres / Years / + * History / Liked / Hidden), matching the web client's library tab bar. + * Genres and Years arrived with #2467; the rest predate it and mirrored + * `flutter_client/lib/library/library_screen.dart`. * * Artists + Albums are wired against the existing LibraryViewModel * (cache-first reads of cached_artists / cached_albums). The other @@ -118,6 +120,12 @@ fun LibraryScreen( when (page) { TAB_ARTISTS -> ArtistsTab(viewModel = viewModel, navController = navController) TAB_ALBUMS -> AlbumsTab(viewModel = viewModel, navController = navController) + TAB_GENRES -> GenresTab( + onAlbumClick = { id -> navController.navigate(AlbumDetail(id)) }, + ) + TAB_YEARS -> YearsTab( + onAlbumClick = { id -> navController.navigate(AlbumDetail(id)) }, + ) TAB_HISTORY -> HistoryTab( onNavigateToAlbum = { id -> navController.navigate(AlbumDetail(id)) }, onNavigateToArtist = { id -> navController.navigate(ArtistDetail(id)) }, @@ -131,11 +139,17 @@ fun LibraryScreen( private const val TAB_ARTISTS = 0 private const val TAB_ALBUMS = 1 -private const val TAB_HISTORY = 2 -private const val TAB_LIKED = 3 -private const val TAB_HIDDEN = 4 +private const val TAB_GENRES = 2 +private const val TAB_YEARS = 3 +private const val TAB_HISTORY = 4 +private const val TAB_LIKED = 5 +private const val TAB_HIDDEN = 6 -private val LIBRARY_TABS = listOf("Artists", "Albums", "History", "Liked", "Hidden") +// Genres and Years sit straight after Albums, matching the web tab bar's +// order (#2467) -- they are browse axes over the same albums, so they belong +// beside them rather than after the personal tabs. +private val LIBRARY_TABS = + listOf("Artists", "Albums", "Genres", "Years", "History", "Liked", "Hidden") @Composable private fun ArtistsTab( diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/YearsTab.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/YearsTab.kt new file mode 100644 index 00000000..b4d53c0a --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/YearsTab.kt @@ -0,0 +1,161 @@ +package com.fabledsword.minstrel.library.ui + +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import androidx.hilt.navigation.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.composables.icons.lucide.Clock +import com.composables.icons.lucide.Lucide +import com.fabledsword.minstrel.shared.UiState +import com.fabledsword.minstrel.shared.widgets.EmptyState +import com.fabledsword.minstrel.shared.widgets.ErrorRetry + +/** + * Years tab (#2467). Same two-state shape as [GenresTab]: the decade-grouped + * index, then the albums for a chosen year. + * + * Albums with no release date are absent from this axis entirely — the server + * leaves them out rather than inventing a year-0 bucket, and the empty state + * says so, because "my albums aren't here" otherwise looks like a bug. + */ +@Composable +fun YearsTab( + onAlbumClick: (String) -> Unit, + viewModel: BrowseViewModel = hiltViewModel(), +) { + val selected by viewModel.selectedYear.collectAsStateWithLifecycle() + val year = selected + if (year == null) { + YearIndex(viewModel = viewModel) + } else { + YearAlbums(year = year, viewModel = viewModel, onAlbumClick = onAlbumClick) + } +} + +@Composable +private fun YearIndex(viewModel: BrowseViewModel) { + val state by viewModel.years.collectAsStateWithLifecycle() + + when (val s = state) { + UiState.Loading -> EmptyState( + title = "Reading release years…", + body = "", + icon = Lucide.Clock, + ) + UiState.Empty -> EmptyState( + title = "No release years found", + body = "Years come from the release date on your albums. Albums " + + "without one don't appear on this axis at all.", + icon = Lucide.Clock, + ) + is UiState.Error -> ErrorRetry( + message = s.message, + onRetry = viewModel::loadYears, + ) + is UiState.Success -> { + val decades = groupByDecade(s.data) + LazyColumn( + modifier = Modifier.fillMaxSize(), + contentPadding = PaddingValues(vertical = 8.dp), + ) { + decades.forEach { group -> + item(key = "decade-${group.decade}") { + DecadeHeader(decade = group.decade, albumCount = group.albumCount) + } + items( + count = group.years.size, + key = { i -> "year-${group.years[i].year}" }, + ) { i -> + val row = group.years[i] + YearRow( + year = row.year, + albumCount = row.albumCount, + onClick = { viewModel.selectYear(row.year) }, + ) + HorizontalDivider() + } + } + } + } + } +} + +@Composable +private fun DecadeHeader(decade: Int, albumCount: Int) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 10.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Text( + text = "${decade}s", + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.primary, + ) + Text( + text = albumCountLabel(albumCount, loading = false, loaded = albumCount), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + +@Composable +private fun YearRow(year: Int, albumCount: Int, onClick: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 14.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = "$year", + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.weight(1f), + ) + Text( + text = "$albumCount", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + +@Composable +private fun YearAlbums( + year: Int, + viewModel: BrowseViewModel, + onAlbumClick: (String) -> Unit, +) { + val albums by viewModel.yearAlbums.collectAsStateWithLifecycle() + BrowseAlbumResults( + heading = "$year", + subtitle = albumCountLabel(albums.total, albums.loading, albums.albums.size), + state = albums, + emptyTitle = "No albums for $year", + emptyBody = "The library may have been rescanned since this list was built.", + onBack = { viewModel.selectYear(null) }, + onRetry = viewModel::retryYearAlbums, + onLoadMore = viewModel::loadMoreYearAlbums, + onAlbumClick = onAlbumClick, + ) +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/BrowseWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/BrowseWire.kt new file mode 100644 index 00000000..346311c1 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/BrowseWire.kt @@ -0,0 +1,33 @@ +package com.fabledsword.minstrel.models.wire + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * One row of `GET /api/library/genres` (#367). + * + * The label is the file tag's own string, split on `[;,]` and trimmed but + * otherwise untouched by the server — no case folding, no synonym mapping. + * So "Rock" and "rock" can both appear, as can "Rock/Pop" beside "Rock" and + * "Pop". Don't normalise it on the client either: the index and the album + * filter have to agree on the exact string, and the filter matches what the + * server stored. + */ +@Serializable +data class GenreCountWire( + val genre: String = "", + @SerialName("track_count") val trackCount: Int = 0, +) + +/** + * One row of `GET /api/library/years`. + * + * Albums with no release date are absent from this axis entirely rather than + * bucketed under year 0 — "unknown" is not a year, and the UI should say so + * instead of showing a fake row. + */ +@Serializable +data class YearCountWire( + val year: Int = 0, + @SerialName("album_count") val albumCount: Int = 0, +) diff --git a/android/app/src/test/java/com/fabledsword/minstrel/library/ui/BrowseIndexTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/library/ui/BrowseIndexTest.kt new file mode 100644 index 00000000..6b3ac788 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/library/ui/BrowseIndexTest.kt @@ -0,0 +1,148 @@ +package com.fabledsword.minstrel.library.ui + +import com.fabledsword.minstrel.library.data.GenreCount +import com.fabledsword.minstrel.library.data.YearCount +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class VisibleGenresTest { + + // Server order: track count descending, name breaking ties. + private val index = listOf( + GenreCount("Rock", 420), + GenreCount("Electronic", 260), + GenreCount("rock", 12), + GenreCount("Alt. Rock", 7), + GenreCount("Rock/Pop", 3), + ) + + @Test + fun `count sort leaves the server's order alone`() { + val out = visibleGenres(index, filter = "", sort = GenreSort.COUNT) + assertEquals(index.map { it.genre }, out.map { it.genre }) + } + + @Test + fun `name sort is alphabetical and case-insensitive`() { + val out = visibleGenres(index, filter = "", sort = GenreSort.NAME) + assertEquals( + listOf("Alt. Rock", "Electronic", "Rock", "rock", "Rock/Pop"), + out.map { it.genre }, + ) + } + + /** + * The input is the list held in the loaded UiState. Sorting it in place + * would reorder what every other reader sees — including the count-sorted + * view the user can toggle straight back to. + */ + @Test + fun `sorting does not mutate the input`() { + val before = index.map { it.genre } + visibleGenres(index, filter = "", sort = GenreSort.NAME) + assertEquals(before, index.map { it.genre }) + } + + @Test + fun `filter matches anywhere in the label, ignoring case`() { + val out = visibleGenres(index, filter = "roCK", sort = GenreSort.COUNT) + assertEquals(listOf("Rock", "rock", "Alt. Rock", "Rock/Pop"), out.map { it.genre }) + } + + @Test + fun `a blank filter is not a filter`() { + assertEquals(index.size, visibleGenres(index, " ", GenreSort.COUNT).size) + } + + @Test + fun `filter and sort compose`() { + val out = visibleGenres(index, filter = "rock", sort = GenreSort.NAME) + assertEquals(listOf("Alt. Rock", "Rock", "rock", "Rock/Pop"), out.map { it.genre }) + } + + @Test + fun `no match yields an empty list rather than everything`() { + assertTrue(visibleGenres(index, "zydeco", GenreSort.COUNT).isEmpty()) + } + + /** + * "Rock/Pop" is a real ID3 tag and the reason the server takes the genre as + * a query parameter. The client must never split or rewrite it — the album + * filter matches the whole stored string. + */ + @Test + fun `a slashed tag survives intact`() { + val out = visibleGenres(index, filter = "Rock/", sort = GenreSort.COUNT) + assertEquals(listOf("Rock/Pop"), out.map { it.genre }) + } +} + +class GroupByDecadeTest { + + @Test + fun `groups by decade, newest decade first`() { + val out = groupByDecade( + listOf( + YearCount(2003, 4), + YearCount(1999, 2), + YearCount(2007, 1), + YearCount(1994, 3), + YearCount(2021, 5), + ), + ) + assertEquals(listOf(2020, 2000, 1990), out.map { it.decade }) + } + + @Test + fun `years within a decade are newest first`() { + val out = groupByDecade( + listOf(YearCount(2003, 1), YearCount(2007, 1), YearCount(2001, 1)), + ) + assertEquals(listOf(2007, 2003, 2001), out.single().years.map { it.year }) + } + + @Test + fun `a decade's album count is the sum of its years`() { + val out = groupByDecade(listOf(YearCount(2003, 4), YearCount(2007, 6))) + assertEquals(10, out.single().albumCount) + } + + @Test + fun `a year on a decade boundary lands in the decade it starts`() { + val out = groupByDecade(listOf(YearCount(2000, 1), YearCount(1999, 1))) + assertEquals(listOf(2000, 1990), out.map { it.decade }) + } + + @Test + fun `an empty index yields no groups`() { + assertTrue(groupByDecade(emptyList()).isEmpty()) + } +} + +class AlbumCountLabelTest { + + /** + * A count that appears as "0 albums" and then corrects itself reads as a + * bug, so the first page's flight shows nothing at all. + */ + @Test + fun `no label while the first page is loading`() { + assertEquals("", albumCountLabel(total = 0, loading = true, loaded = 0)) + } + + @Test + fun `a later page keeps showing the known total`() { + assertEquals("40 albums", albumCountLabel(total = 40, loading = true, loaded = 20)) + } + + @Test + fun `singular is not pluralised`() { + assertEquals("1 album", albumCountLabel(total = 1, loading = false, loaded = 1)) + } + + @Test + fun `a genuinely empty result says zero`() { + assertEquals("0 albums", albumCountLabel(total = 0, loading = false, loaded = 0)) + } +}