From d55406c85e191cbc7edc5e570a087666d89333a6 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:58:35 +0000 Subject: [PATCH] address review: keep per-tab search visible and clearable, fix direction arrow, guard scroll reset, harden date parsing --- .../data/source/subsonic/SubsonicXml.kt | 9 +++-- .../main/kotlin/com/subtracks/ui/AppRoot.kt | 1 - .../subtracks/ui/components/ViewportFill.kt | 5 ++- .../com/subtracks/ui/library/LibraryScreen.kt | 34 +++++++++++-------- .../ui/LibraryOptionsScreenshotTest.kt | 1 - 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt index 7094129e..16db1a4b 100644 --- a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt +++ b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicXml.kt @@ -134,15 +134,20 @@ private fun Element.dateAttr(name: String): Long? = getAttribute(name).takeIf { internal object IsoDate { private val timezone = Regex("(Z|[+-]\\d{2}:?\\d{2})$", RegexOption.IGNORE_CASE) + private val longFraction = Regex("\\.(\\d{9})\\d+") fun parse(value: String): Long? { - val text = value.trim().let { if (it.endsWith("z")) it.dropLast(1) + "Z" else it } + val text = + value + .trim() + .let { if (it.endsWith("z")) it.dropLast(1) + "Z" else it } + .replace(longFraction) { ".${it.groupValues[1]}" } return try { val zoned = if (timezone.containsMatchIn(text)) text else text + "Z" OffsetDateTime.parse(zoned).toEpochSecond() } catch (_: DateTimeParseException) { try { - LocalDate.parse(text).atStartOfDay(ZoneOffset.UTC).toEpochSecond() + LocalDate.parse(text.removeSuffix("Z")).atStartOfDay(ZoneOffset.UTC).toEpochSecond() } catch (_: DateTimeParseException) { null } diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index 5f5eb3a7..fd35c8b6 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -22,7 +22,6 @@ import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.ime import androidx.compose.foundation.layout.imeAnimationTarget import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.navigationBars diff --git a/app/src/main/kotlin/com/subtracks/ui/components/ViewportFill.kt b/app/src/main/kotlin/com/subtracks/ui/components/ViewportFill.kt index 8bcac2b9..6a4e68c9 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ViewportFill.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ViewportFill.kt @@ -30,7 +30,10 @@ fun ResetScrollOnChange( onReset: suspend () -> Unit, ) { var pending by remember { mutableStateOf(false) } - LaunchedEffect(resetKey) { pending = true } + var primed by remember { mutableStateOf(false) } + LaunchedEffect(resetKey) { + if (primed) pending = true else primed = true + } LaunchedEffect(Unit) { var refreshes = 0 var handled = 0 diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 552ec3e1..b62b58e7 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -14,7 +14,6 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.imePadding -import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size @@ -115,6 +114,7 @@ enum class LibraryTab( data class SortOption( val value: String, val label: String, + val descendingByDefault: Boolean = false, ) fun LibraryTab.listTab(): LibraryListTab = LibraryListTab.valueOf(name) @@ -125,25 +125,25 @@ fun sortOptionsFor(tab: LibraryTab): List = listOf( SortOption(AlbumSort.Name.name, "Name"), SortOption(AlbumSort.Artist.name, "Artist"), - SortOption(AlbumSort.Year.name, "Year"), - SortOption(AlbumSort.Added.name, "Added"), - SortOption(AlbumSort.Starred.name, "Starred"), + SortOption(AlbumSort.Year.name, "Year", descendingByDefault = true), + SortOption(AlbumSort.Added.name, "Added", descendingByDefault = true), + SortOption(AlbumSort.Starred.name, "Starred", descendingByDefault = true), ) } LibraryTab.Artists -> { listOf( SortOption(ArtistSort.Name.name, "Name"), - SortOption(ArtistSort.AlbumCount.name, "Albums"), - SortOption(ArtistSort.Starred.name, "Starred"), + SortOption(ArtistSort.AlbumCount.name, "Albums", descendingByDefault = true), + SortOption(ArtistSort.Starred.name, "Starred", descendingByDefault = true), ) } LibraryTab.Playlists -> { listOf( SortOption(PlaylistSort.Name.name, "Name"), - SortOption(PlaylistSort.Added.name, "Added"), - SortOption(PlaylistSort.Updated.name, "Updated"), + SortOption(PlaylistSort.Added.name, "Added", descendingByDefault = true), + SortOption(PlaylistSort.Updated.name, "Updated", descendingByDefault = true), ) } @@ -152,8 +152,8 @@ fun sortOptionsFor(tab: LibraryTab): List = SortOption(SongSort.Album.name, "Album"), SortOption(SongSort.Title.name, "Title"), SortOption(SongSort.Artist.name, "Artist"), - SortOption(SongSort.Starred.name, "Starred"), - SortOption(SongSort.Added.name, "Added"), + SortOption(SongSort.Starred.name, "Starred", descendingByDefault = true), + SortOption(SongSort.Added.name, "Added", descendingByDefault = true), ) } } @@ -271,7 +271,7 @@ fun LibraryScreen( if (pagerState.currentPage != selectedTab.ordinal) { pagerState.animateScrollToPage(selectedTab.ordinal) } - searchActive = false + searchActive = search.isNotEmpty() } val density = LocalDensity.current @@ -418,7 +418,7 @@ fun LibraryScreen( ) { Box { Icon(Icons.Filled.Sort, contentDescription = "List options") - if (listQuery.starred != StarredFilter.Any) { + if (filtersActive) { Box( modifier = Modifier @@ -440,6 +440,7 @@ fun LibraryScreen( listQuery = listQuery, sortOptions = sortOptions, starredSupported = starredSupported, + canClearFilters = filtersActive, onSortChange = onSortChange, onToggleSortDirection = onToggleSortDirection, onCycleStarred = onCycleStarred, @@ -459,6 +460,7 @@ private fun ListOptionsSheet( listQuery: ListQuery, sortOptions: List, starredSupported: Boolean, + canClearFilters: Boolean, onSortChange: (String) -> Unit, onToggleSortDirection: () -> Unit, onCycleStarred: () -> Unit, @@ -466,6 +468,8 @@ private fun ListOptionsSheet( onSearch: () -> Unit, ) { Column(Modifier.fillMaxWidth().padding(bottom = 24.dp)) { + val naturalDescending = sortOptions.firstOrNull { it.value == listQuery.sort }?.descendingByDefault == true + val descending = if (naturalDescending) !listQuery.descending else listQuery.descending Surface( onClick = onSearch, shape = RoundedCornerShape(4.dp), @@ -495,8 +499,8 @@ private fun ListOptionsSheet( ) IconButton(onClick = onToggleSortDirection) { Icon( - imageVector = if (listQuery.descending) Icons.Rounded.ArrowDownward else Icons.Rounded.ArrowUpward, - contentDescription = if (listQuery.descending) "Sort descending" else "Sort ascending", + imageVector = if (descending) Icons.Rounded.ArrowDownward else Icons.Rounded.ArrowUpward, + contentDescription = if (descending) "Sort descending" else "Sort ascending", ) } } @@ -528,7 +532,7 @@ private fun ListOptionsSheet( ) IconButton( onClick = onClearFilters, - enabled = listQuery.starred != StarredFilter.Any, + enabled = canClearFilters, ) { Icon(Icons.Rounded.FilterAltOff, contentDescription = "Clear filters") } diff --git a/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsScreenshotTest.kt index ceb8350a..cd3cd96b 100644 --- a/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsScreenshotTest.kt @@ -2,7 +2,6 @@ package com.subtracks.ui import androidx.activity.ComponentActivity import androidx.compose.runtime.remember -import androidx.compose.ui.Modifier import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onNodeWithContentDescription