fix(library): improve sorting and grouping interaction, closes #3517 (#3524)

- Sort author groups by last name instead of first name when sorting by author
- Sort series groups by book author instead of series name when sorting by author
- Place ungrouped books before custom groups instead of mixing them together

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-03-12 23:59:50 +08:00
committed by GitHub
parent 674a0bf16d
commit f9a3559086
3 changed files with 97 additions and 11 deletions
@@ -385,7 +385,30 @@ describe('getGroupSortValue', () => {
expect(getGroupSortValue(group, LibrarySortByType.Title)).toBe('My Series');
});
it('should return group name for author sort', () => {
it('should return last-name-first for author group sorted by author', () => {
const group = createMockGroup({
name: 'Terry Pratchett',
books: [createMockBook({ author: 'Terry Pratchett' })],
});
expect(getGroupSortValue(group, LibrarySortByType.Author, LibraryGroupByType.Author)).toBe(
'Pratchett, Terry',
);
});
it('should return book author for series group sorted by author', () => {
const group = createMockGroup({
name: 'Discworld',
books: [
createMockBook({ author: 'Terry Pratchett', primaryLanguage: 'en' }),
createMockBook({ author: 'Terry Pratchett', primaryLanguage: 'en' }),
],
});
expect(getGroupSortValue(group, LibrarySortByType.Author, LibraryGroupByType.Series)).toBe(
'Pratchett, Terry',
);
});
it('should fallback to group name for author sort without groupBy', () => {
const group = createMockGroup({ name: 'John Smith' });
expect(getGroupSortValue(group, LibrarySortByType.Author)).toBe('John Smith');
});
@@ -536,6 +559,51 @@ describe('createGroupSorter', () => {
expect(sorted[0]!.name).toBe('Group A');
expect(sorted[1]!.name).toBe('Group B');
});
it('should sort series groups by book author when sorting by author', () => {
const groups = [
createMockGroup({
name: 'Discworld',
books: [createMockBook({ author: 'Terry Pratchett', primaryLanguage: 'en' })],
}),
createMockGroup({
name: 'Foundation',
books: [createMockBook({ author: 'Isaac Asimov', primaryLanguage: 'en' })],
}),
];
const sorter = createGroupSorter(LibrarySortByType.Author, 'en', LibraryGroupByType.Series);
const sorted = [...groups].sort(sorter);
// Asimov < Pratchett (by last name)
expect(sorted[0]!.name).toBe('Foundation');
expect(sorted[1]!.name).toBe('Discworld');
});
it('should sort author groups by last name when sorting by author', () => {
const groups = [
createMockGroup({
name: 'Terry Pratchett',
books: [createMockBook({ author: 'Terry Pratchett' })],
}),
createMockGroup({
name: 'Umberto Eco',
books: [createMockBook({ author: 'Umberto Eco' })],
}),
createMockGroup({
name: 'Isaac Asimov',
books: [createMockBook({ author: 'Isaac Asimov' })],
}),
];
const sorter = createGroupSorter(LibrarySortByType.Author, 'en', LibraryGroupByType.Author);
const sorted = [...groups].sort(sorter);
// Last names: Asimov, Eco, Pratchett
expect(sorted[0]!.name).toBe('Isaac Asimov');
expect(sorted[1]!.name).toBe('Umberto Eco');
expect(sorted[2]!.name).toBe('Terry Pratchett');
});
});
describe('createBookSorter', () => {
@@ -202,7 +202,7 @@ const Bookshelf: React.FC<BookshelfProps> = ({
// Merge groups and ungrouped books, then sort them together
const allItems: (Book | BooksGroup)[] = [...groups, ...ungroupedBooks];
const groupSorter = createGroupSorter(sortBy, uiLanguage);
const groupSorter = createGroupSorter(sortBy, uiLanguage, groupBy);
allItems.sort((a, b) => {
const isAGroup = 'books' in a;
@@ -218,14 +218,20 @@ const Bookshelf: React.FC<BookshelfProps> = ({
return bookSorter(a, b) * sortOrderMultiplier;
}
// One is a group, one is a book - compare their sort values
// One is a group, one is a book
if (groupBy === LibraryGroupByType.Group) {
// For custom groups: always place ungrouped books before groups
return isAGroup ? 1 : -1;
}
// For series/author groups: compare sort values to interleave properly
if (isAGroup && !isBGroup) {
const groupValue = getGroupSortValue(a, sortBy);
const groupValue = getGroupSortValue(a, sortBy, groupBy);
const bookValue = getBookSortValue(b, sortBy);
return compareSortValues(groupValue, bookValue, uiLanguage) * sortOrderMultiplier;
} else if (!isAGroup && isBGroup) {
const bookValue = getBookSortValue(a, sortBy);
const groupValue = getGroupSortValue(b, sortBy);
const groupValue = getGroupSortValue(b, sortBy, groupBy);
return compareSortValues(bookValue, groupValue, uiLanguage) * sortOrderMultiplier;
}
return 0;
@@ -335,6 +335,7 @@ export const getBookSortValue = (book: Book, sortBy: LibrarySortByType): number
export const getGroupSortValue = (
group: BooksGroup,
sortBy: LibrarySortByType,
groupBy?: LibraryGroupByType,
): number | string => {
const books = group.books;
@@ -348,12 +349,23 @@ export const getGroupSortValue = (
switch (sortBy) {
case LibrarySortByType.Title:
case LibrarySortByType.Author:
case LibrarySortByType.Format:
// For text-based sorts, use the group name
// This isn't perfect, especially with the
return group.name;
case LibrarySortByType.Author: {
if (groupBy === LibraryGroupByType.Author) {
// Author group: format the group name (single author) with last-name-first
return formatAuthors(group.name, 'en', true);
}
if (groupBy === LibraryGroupByType.Series) {
// Series group: use the first book's author for sorting
const firstBook = books[0]!;
return formatAuthors(firstBook.author, firstBook.primaryLanguage || 'en', true);
}
// Custom/other groups: fall back to group name
return group.name;
}
case LibrarySortByType.Updated:
// Return the most recent updatedAt
return Math.max(...books.map((b) => b.updatedAt));
@@ -403,10 +415,10 @@ export const compareSortValues = (
* Create a sorter for groups themselves based on sort criteria.
*/
export const createGroupSorter =
(sortBy: LibrarySortByType, uiLanguage: string) =>
(sortBy: LibrarySortByType, uiLanguage: string, groupBy?: LibraryGroupByType) =>
(a: BooksGroup, b: BooksGroup): number => {
const aValue = getGroupSortValue(a, sortBy);
const bValue = getGroupSortValue(b, sortBy);
const aValue = getGroupSortValue(a, sortBy, groupBy);
const bValue = getGroupSortValue(b, sortBy, groupBy);
// String comparison for text-based sorts
if (typeof aValue === 'string' && typeof bValue === 'string') {