mirror of
https://github.com/JustinZeus/Wholphin.git
synced 2026-11-05 18:30:59 +01:00
Fix alphabet highlight for names starting with "The " (#1788)
## Description The alphabet scroll bar highlighted the letter of an item's *display* name while the grid sorted by the server's article-stripped `SortName`, so "The Beatles" sat under **B** but highlighted **T**. - **Artists:** the Artists tab never requested `SortName`, so the highlight silently fell back to `Name`. It now asks for `SlimItemFields` (which already includes `SORT_NAME`), matching the sibling `/Items` branch. The count-only query used for letter positions passes `fields = null`, since a `limit = 0` request has no use for them. - **Genres and studios:** these grids bucketed on the display name unconditionally. The sort-name fallback is extracted as `BaseItemDto.alphabetSortName` (`sortName ?: name ?: ""`) and carried onto the `Genre` and `Studio` grid items, so all three grids now derive the letter from one value. ### Related issues https://github.com/damontecres/Wholphin/issues/1787 ### Testing Two unit tests added: - `CollectionFolderViewModelTest` pins that the artist pager actually requests `SORT_NAME` — the field whose absence caused the bug. - `AlphabetSortNameTest` covers both branches of the `alphabetSortName` fallback. Manually: select a music library → Artists tab → put the cursor on an artist starting with "The " and confirm the highlighted letter matches the grid position. Same check on the genre and studio grids. ## AI or LLM usage I used Cursor for debugging and implementing the fix, and Claude Code for a follow-up cleanup pass over the diff.
This commit is contained in:
parent
454c0212b6
commit
a0966dd05a
6 changed files with 73 additions and 5 deletions
|
|
@ -54,7 +54,7 @@ data class BaseItem(
|
|||
get() = type.playable
|
||||
|
||||
override val sortName: String
|
||||
get() = data.sortName ?: data.name ?: ""
|
||||
get() = data.alphabetSortName
|
||||
|
||||
val type get() = data.type
|
||||
|
||||
|
|
@ -305,6 +305,9 @@ data class BaseItem(
|
|||
|
||||
val BaseItemDto.aspectRatioFloat: Float? get() = width?.let { w -> height?.let { h -> w.toFloat() / h.toFloat() } }
|
||||
|
||||
/** The server strips leading articles (eg "The ") from [BaseItemDto.sortName]. */
|
||||
val BaseItemDto.alphabetSortName: String get() = sortName ?: name ?: ""
|
||||
|
||||
@Immutable
|
||||
data class BaseItemUi(
|
||||
val episodeCornerText: String?,
|
||||
|
|
|
|||
|
|
@ -399,6 +399,7 @@ class CollectionFolderViewModel
|
|||
GetArtistsRequest(
|
||||
parentId = item?.id,
|
||||
enableImageTypes = listOf(ImageType.PRIMARY, ImageType.THUMB),
|
||||
fields = SlimItemFields,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
|
@ -477,6 +478,7 @@ class CollectionFolderViewModel
|
|||
GetArtistsHandler.countMatching(
|
||||
createGetArtistsRequest(filter).copy(
|
||||
enableImageTypes = null,
|
||||
fields = null,
|
||||
nameLessThan = letter.toString(),
|
||||
limit = 0,
|
||||
enableTotalRecordCount = true,
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import androidx.lifecycle.ViewModel
|
|||
import androidx.lifecycle.viewModelScope
|
||||
import com.github.damontecres.wholphin.data.ServerRepository
|
||||
import com.github.damontecres.wholphin.data.model.BaseItem
|
||||
import com.github.damontecres.wholphin.data.model.alphabetSortName
|
||||
import com.github.damontecres.wholphin.data.model.createGenreDestination
|
||||
import com.github.damontecres.wholphin.services.ImageUrlService
|
||||
import com.github.damontecres.wholphin.services.NavigationManager
|
||||
|
|
@ -104,7 +105,12 @@ class GenreViewModel
|
|||
.execute(api, request)
|
||||
.content.items
|
||||
.map {
|
||||
Genre(it.id, it.name ?: "", null)
|
||||
Genre(
|
||||
id = it.id,
|
||||
name = it.name ?: "",
|
||||
imageUrl = null,
|
||||
sortName = it.alphabetSortName,
|
||||
)
|
||||
}
|
||||
_state.update {
|
||||
it.copy(
|
||||
|
|
@ -243,10 +249,10 @@ data class Genre(
|
|||
val id: UUID,
|
||||
val name: String,
|
||||
val imageUrl: String?,
|
||||
override val sortName: String = name,
|
||||
) : CardGridItem {
|
||||
override val gridId: String get() = id.toString()
|
||||
override val playable: Boolean = false
|
||||
override val sortName: String get() = name
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import androidx.lifecycle.ViewModel
|
|||
import androidx.lifecycle.viewModelScope
|
||||
import com.github.damontecres.wholphin.data.ServerRepository
|
||||
import com.github.damontecres.wholphin.data.model.BaseItem
|
||||
import com.github.damontecres.wholphin.data.model.alphabetSortName
|
||||
import com.github.damontecres.wholphin.data.model.createStudioDestination
|
||||
import com.github.damontecres.wholphin.services.ImageUrlService
|
||||
import com.github.damontecres.wholphin.services.NavigationManager
|
||||
|
|
@ -97,7 +98,12 @@ class StudioViewModel
|
|||
imageType = ImageType.THUMB,
|
||||
fillWidth = cardWidthPx,
|
||||
)
|
||||
Studio(it.id, it.name ?: "", imageUrl)
|
||||
Studio(
|
||||
id = it.id,
|
||||
name = it.name ?: "",
|
||||
imageUrl = imageUrl,
|
||||
sortName = it.alphabetSortName,
|
||||
)
|
||||
}
|
||||
_state.update {
|
||||
it.copy(
|
||||
|
|
@ -133,10 +139,10 @@ data class Studio(
|
|||
val id: UUID,
|
||||
val name: String,
|
||||
val imageUrl: String?,
|
||||
override val sortName: String = name,
|
||||
) : CardGridItem {
|
||||
override val gridId: String get() = id.toString()
|
||||
override val playable: Boolean = false
|
||||
override val sortName: String get() = name
|
||||
}
|
||||
|
||||
data class StudioGridState(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,36 @@
|
|||
package com.github.damontecres.wholphin.data.model
|
||||
|
||||
import org.jellyfin.sdk.model.api.BaseItemDto
|
||||
import org.jellyfin.sdk.model.api.BaseItemKind
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
import java.util.UUID
|
||||
|
||||
class AlphabetSortNameTest {
|
||||
@Test
|
||||
fun prefersSortNameOverName() {
|
||||
val dto =
|
||||
BaseItemDto(
|
||||
id = UUID.randomUUID(),
|
||||
type = BaseItemKind.MUSIC_ARTIST,
|
||||
name = "The Beatles",
|
||||
sortName = "beatles",
|
||||
)
|
||||
|
||||
assertEquals("beatles", dto.alphabetSortName)
|
||||
assertEquals("beatles", BaseItem(dto).sortName)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun fallsBackToNameWhenSortNameMissing() {
|
||||
val dto =
|
||||
BaseItemDto(
|
||||
id = UUID.randomUUID(),
|
||||
type = BaseItemKind.STUDIO,
|
||||
name = "The Walt Disney Company",
|
||||
sortName = null,
|
||||
)
|
||||
|
||||
assertEquals("The Walt Disney Company", dto.alphabetSortName)
|
||||
}
|
||||
}
|
||||
|
|
@ -33,10 +33,12 @@ import org.jellyfin.sdk.api.operations.UserLibraryApi
|
|||
import org.jellyfin.sdk.model.api.BaseItemDto
|
||||
import org.jellyfin.sdk.model.api.BaseItemKind
|
||||
import org.jellyfin.sdk.model.api.CollectionType
|
||||
import org.jellyfin.sdk.model.api.ItemFields
|
||||
import org.jellyfin.sdk.model.api.request.GetArtistsRequest
|
||||
import org.jellyfin.sdk.model.api.request.GetItemsRequest
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import java.util.UUID
|
||||
|
|
@ -141,6 +143,19 @@ class CollectionFolderViewModelTest {
|
|||
coVerify(exactly = 0) { GetItemsRequestHandler.execute(any(), any<GetItemsRequest>()) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Without SortName, the alphabet highlight falls back to Name and highlights "T" for artists
|
||||
* like "The Beatles" even though the grid sorts them under "B".
|
||||
*/
|
||||
@Test
|
||||
fun `artist pager requests SortName for alphabet highlighting`() =
|
||||
runTest(testDispatcher) {
|
||||
createViewModel(GetItemsFilter(override = GetItemsFilterOverride.ARTIST))
|
||||
advanceUntilIdle()
|
||||
|
||||
assertTrue(ItemFields.SORT_NAME in artistsRequest.captured.fields.orEmpty())
|
||||
}
|
||||
|
||||
/**
|
||||
* /Persons has no nameLessThan or startIndex, so counting up to a letter is not expressible.
|
||||
* Falling back to /Items would count media rather than people, ie the same defect as the artist
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue