From f9a355908688d5877f3ce62ed7242d4f1c6b8995 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 12 Mar 2026 23:59:50 +0800 Subject: [PATCH] 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 --- .../src/__tests__/utils/library-utils.test.ts | 70 ++++++++++++++++++- .../src/app/library/components/Bookshelf.tsx | 14 ++-- .../src/app/library/utils/libraryUtils.ts | 24 +++++-- 3 files changed, 97 insertions(+), 11 deletions(-) diff --git a/apps/readest-app/src/__tests__/utils/library-utils.test.ts b/apps/readest-app/src/__tests__/utils/library-utils.test.ts index 35ceef30..2c1f72a5 100644 --- a/apps/readest-app/src/__tests__/utils/library-utils.test.ts +++ b/apps/readest-app/src/__tests__/utils/library-utils.test.ts @@ -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', () => { diff --git a/apps/readest-app/src/app/library/components/Bookshelf.tsx b/apps/readest-app/src/app/library/components/Bookshelf.tsx index 62d47f99..7c9d51f3 100644 --- a/apps/readest-app/src/app/library/components/Bookshelf.tsx +++ b/apps/readest-app/src/app/library/components/Bookshelf.tsx @@ -202,7 +202,7 @@ const Bookshelf: React.FC = ({ // 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 = ({ 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; diff --git a/apps/readest-app/src/app/library/utils/libraryUtils.ts b/apps/readest-app/src/app/library/utils/libraryUtils.ts index 72b90b72..8ff4dba6 100644 --- a/apps/readest-app/src/app/library/utils/libraryUtils.ts +++ b/apps/readest-app/src/app/library/utils/libraryUtils.ts @@ -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') {