diff --git a/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts b/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts new file mode 100644 index 00000000..044bf04d --- /dev/null +++ b/apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from 'vitest'; + +import { getBookContextMenuItemIds } from '@/app/library/utils/libraryUtils'; +import { Book } from '@/types/book'; + +const createBook = (overrides: Partial = {}): Book => ({ + hash: 'hash-1', + format: 'EPUB', + title: 'Test Book', + author: 'Test Author', + createdAt: 0, + updatedAt: 0, + ...overrides, +}); + +describe('getBookContextMenuItemIds', () => { + it('returns a deterministic order for a local downloaded book', () => { + const book = createBook({ downloadedAt: 1 }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markFinished', + 'showDetails', + 'showInFinder', + 'upload', + 'share', + 'delete', + ]); + }); + + it('shows "Mark as Unread" + "Clear Status" for a finished book', () => { + const book = createBook({ downloadedAt: 1, readingStatus: 'finished' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markUnread', + 'clearStatus', + 'showDetails', + 'showInFinder', + 'upload', + 'share', + 'delete', + ]); + }); + + it('shows "Mark as Finished" + "Clear Status" for an unread book', () => { + const book = createBook({ downloadedAt: 1, readingStatus: 'unread' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markFinished', + 'clearStatus', + 'showDetails', + 'showInFinder', + 'upload', + 'share', + 'delete', + ]); + }); + + it('offers Download (not Upload) for a cloud-only book', () => { + const book = createBook({ uploadedAt: 1 }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markFinished', + 'showDetails', + 'showInFinder', + 'download', + 'share', + 'delete', + ]); + }); + + it('omits download/upload/share for a book that is neither downloaded nor uploaded', () => { + const book = createBook({ filePath: '/some/external/file.epub' }); + expect(getBookContextMenuItemIds(book)).toEqual([ + 'select', + 'group', + 'markFinished', + 'showDetails', + 'showInFinder', + 'delete', + ]); + }); + + it('produces the same order on repeated calls and never duplicates an item (issue #4389)', () => { + const book = createBook({ downloadedAt: 1, uploadedAt: 1, readingStatus: 'finished' }); + const first = getBookContextMenuItemIds(book); + const second = getBookContextMenuItemIds(book); + expect(second).toEqual(first); + expect(new Set(first).size).toBe(first.length); + }); +}); diff --git a/apps/readest-app/src/app/library/components/BookshelfItem.tsx b/apps/readest-app/src/app/library/components/BookshelfItem.tsx index 69075d64..fbf3c86f 100644 --- a/apps/readest-app/src/app/library/components/BookshelfItem.tsx +++ b/apps/readest-app/src/app/library/components/BookshelfItem.tsx @@ -6,7 +6,7 @@ import { useSettingsStore } from '@/store/settingsStore'; import { useTranslation } from '@/hooks/useTranslation'; import { useAppRouter } from '@/hooks/useAppRouter'; import { useLongPress } from '@/hooks/useLongPress'; -import { Menu, MenuItem } from '@tauri-apps/api/menu'; +import { Menu, type MenuItemOptions } from '@tauri-apps/api/menu'; import { revealItemInDir } from '@tauri-apps/plugin-opener'; import { eventDispatcher } from '@/utils/event'; import { getOSPlatform } from '@/utils/misc'; @@ -16,6 +16,10 @@ import { LibraryCoverFitType, LibraryViewModeType } from '@/types/settings'; import { BOOK_UNGROUPED_ID, BOOK_UNGROUPED_NAME } from '@/services/constants'; import { FILE_REVEAL_LABELS, FILE_REVEAL_PLATFORMS } from '@/utils/os'; import { Book, BooksGroup, ReadingStatus } from '@/types/book'; +import { + getBookContextMenuItemIds, + type BookContextMenuItemId, +} from '@/app/library/utils/libraryUtils'; import { md5Fingerprint } from '@/utils/md5'; import BookItem from './BookItem'; import GroupItem from './GroupItem'; @@ -214,144 +218,127 @@ const BookshelfItem: React.FC = ({ const osPlatform = getOSPlatform(); const fileRevealLabel = FILE_REVEAL_LABELS[osPlatform as FILE_REVEAL_PLATFORMS] || FILE_REVEAL_LABELS.default; - const selectBookMenuItem = await MenuItem.new({ - text: itemSelected ? _('Deselect Book') : _('Select Book'), - action: async () => { - if (!isSelectMode) handleSetSelectMode(true); - toggleSelection(book.hash); - }, - }); - const groupBooksMenuItem = await MenuItem.new({ - text: _('Group Books'), - action: async () => { - if (!isSelectMode) handleSetSelectMode(true); - if (!itemSelected) { + // Build every item up front, then create the menu from the ordered subset + // in a single Menu.new({ items }) call. Appending items one-by-one with + // un-awaited Menu.append() promises races on the Tauri IPC boundary and + // shuffles the order on every open (issue #4389). + const itemOptions: Record = { + select: { + text: itemSelected ? _('Deselect Book') : _('Select Book'), + action: async () => { + if (!isSelectMode) handleSetSelectMode(true); toggleSelection(book.hash); - } - handleGroupBooks(); + }, }, - }); - const markAsFinishedMenuItem = await MenuItem.new({ - text: _('Mark as Finished'), - action: async () => { - handleUpdateReadingStatus(book, 'finished'); + group: { + text: _('Group Books'), + action: async () => { + if (!isSelectMode) handleSetSelectMode(true); + if (!itemSelected) { + toggleSelection(book.hash); + } + handleGroupBooks(); + }, }, - }); - const markAsUnreadMenuItem = await MenuItem.new({ - text: _('Mark as Unread'), - action: async () => { - handleUpdateReadingStatus(book, 'unread'); + markFinished: { + text: _('Mark as Finished'), + action: async () => { + handleUpdateReadingStatus(book, 'finished'); + }, }, - }); - const clearStatusMenuItem = await MenuItem.new({ - text: _('Clear Status'), - action: async () => { - handleUpdateReadingStatus(book, undefined); + markUnread: { + text: _('Mark as Unread'), + action: async () => { + handleUpdateReadingStatus(book, 'unread'); + }, }, - }); - const showBookInFinderMenuItem = await MenuItem.new({ - text: _(fileRevealLabel), - action: async () => { - const folder = `${settings.localBooksDir}/${book.hash}`; - revealItemInDir(folder); + clearStatus: { + text: _('Clear Status'), + action: async () => { + handleUpdateReadingStatus(book, undefined); + }, }, - }); - const showBookDetailsMenuItem = await MenuItem.new({ - text: _('Show Book Details'), - action: async () => { - showBookDetailsModal(book); + showDetails: { + text: _('Show Book Details'), + action: async () => { + showBookDetailsModal(book); + }, }, - }); - const downloadBookMenuItem = await MenuItem.new({ - text: _('Download Book'), - action: async () => { - handleBookDownload(book, { queued: true }); + showInFinder: { + text: _(fileRevealLabel), + action: async () => { + const folder = `${settings.localBooksDir}/${book.hash}`; + revealItemInDir(folder); + }, }, - }); - const uploadBookMenuItem = await MenuItem.new({ - text: _('Upload Book'), - action: async () => { - handleBookUpload(book); + download: { + text: _('Download Book'), + action: async () => { + handleBookDownload(book, { queued: true }); + }, }, - }); - const shareBookMenuItem = await MenuItem.new({ - text: _('Share Book'), - action: async () => { - // Bookshelf.tsx hosts the dialog; we dispatch and let it route - // unauthenticated users into the login flow first. - eventDispatcher.dispatch('show-share-dialog', { book }); + upload: { + text: _('Upload Book'), + action: async () => { + handleBookUpload(book); + }, }, - }); - const deleteBookMenuItem = await MenuItem.new({ - text: _('Delete'), - action: async () => { - eventDispatcher.dispatch('delete-books', { ids: [book.hash] }); + share: { + text: _('Share Book'), + action: async () => { + // Bookshelf.tsx hosts the dialog; we dispatch and let it route + // unauthenticated users into the login flow first. + eventDispatcher.dispatch('show-share-dialog', { book }); + }, }, - }); - const menu = await Menu.new(); - menu.append(selectBookMenuItem); - menu.append(groupBooksMenuItem); - if (book.readingStatus === 'finished') { - menu.append(markAsUnreadMenuItem); - } else { - menu.append(markAsFinishedMenuItem); - } - // show "Clear Status" option when book has an explicit status set - if (book.readingStatus === 'finished' || book.readingStatus === 'unread') { - menu.append(clearStatusMenuItem); - } - menu.append(showBookDetailsMenuItem); - menu.append(showBookInFinderMenuItem); - if (book.uploadedAt && !book.downloadedAt) { - menu.append(downloadBookMenuItem); - } - if (!book.uploadedAt && book.downloadedAt) { - menu.append(uploadBookMenuItem); - } - // Share is offered for any local-or-uploaded book; the dialog will trigger - // an upload first if the book hasn't been pushed yet. - if (book.downloadedAt || book.uploadedAt) { - menu.append(shareBookMenuItem); - } - menu.append(deleteBookMenuItem); - menu.popup(); + delete: { + text: _('Delete'), + action: async () => { + eventDispatcher.dispatch('delete-books', { ids: [book.hash] }); + }, + }, + }; + const items = getBookContextMenuItemIds(book).map((id) => itemOptions[id]); + const menu = await Menu.new({ items }); + await menu.popup(); }; const groupContextMenuHandler = async (group: BooksGroup) => { if (!appService?.hasContextMenu) return; - const selectGroupMenuItem = await MenuItem.new({ - text: itemSelected ? _('Deselect Group') : _('Select Group'), - action: async () => { - if (!isSelectMode) handleSetSelectMode(true); - toggleSelection(group.id); - }, - }); - const groupBooksMenuItem = await MenuItem.new({ - text: _('Group Books'), - action: async () => { - if (!isSelectMode) handleSetSelectMode(true); - if (!itemSelected) { + // Single Menu.new({ items }) call keeps the order deterministic — see the + // note in bookContextMenuHandler about the Menu.append() IPC race (#4389). + const items: MenuItemOptions[] = [ + { + text: itemSelected ? _('Deselect Group') : _('Select Group'), + action: async () => { + if (!isSelectMode) handleSetSelectMode(true); toggleSelection(group.id); - } - handleGroupBooks(); + }, }, - }); - const deleteGroupMenuItem = await MenuItem.new({ - text: _('Delete'), - action: async () => { - // Dispatch the constituent book hashes — `group.books` is the - // rendered rollup and already includes books from nested sub- - // folders, so the deletion path doesn't need to re-derive what - // belongs to the group from the id alone. - const ids = group.books.filter((book) => !book.deletedAt).map((book) => book.hash); - eventDispatcher.dispatch('delete-books', { ids }); + { + text: _('Group Books'), + action: async () => { + if (!isSelectMode) handleSetSelectMode(true); + if (!itemSelected) { + toggleSelection(group.id); + } + handleGroupBooks(); + }, }, - }); - const menu = await Menu.new(); - menu.append(selectGroupMenuItem); - menu.append(groupBooksMenuItem); - menu.append(deleteGroupMenuItem); - menu.popup(); + { + text: _('Delete'), + action: async () => { + // Dispatch the constituent book hashes — `group.books` is the + // rendered rollup and already includes books from nested sub- + // folders, so the deletion path doesn't need to re-derive what + // belongs to the group from the id alone. + const ids = group.books.filter((book) => !book.deletedAt).map((book) => book.hash); + eventDispatcher.dispatch('delete-books', { ids }); + }, + }, + ]; + const menu = await Menu.new({ items }); + await menu.popup(); }; // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/apps/readest-app/src/app/library/utils/libraryUtils.ts b/apps/readest-app/src/app/library/utils/libraryUtils.ts index 94dfa93c..4ca15d33 100644 --- a/apps/readest-app/src/app/library/utils/libraryUtils.ts +++ b/apps/readest-app/src/app/library/utils/libraryUtils.ts @@ -583,3 +583,41 @@ export const createGroupSorter = return 0; }; + +export type BookContextMenuItemId = + | 'select' + | 'group' + | 'markFinished' + | 'markUnread' + | 'clearStatus' + | 'showDetails' + | 'showInFinder' + | 'download' + | 'upload' + | 'share' + | 'delete'; + +/** + * Resolve the ordered list of context-menu item ids for a book from its state. + * + * The native menu MUST be built from this list in a single `Menu.new({ items })` + * call. Appending items one at a time with un-awaited `Menu.append()` promises + * races on the Tauri IPC boundary, so the items land in a non-deterministic + * order and the menu appears to shuffle on every open (issue #4389). + */ +export const getBookContextMenuItemIds = (book: Book): BookContextMenuItemId[] => { + const ids: BookContextMenuItemId[] = ['select', 'group']; + ids.push(book.readingStatus === 'finished' ? 'markUnread' : 'markFinished'); + // "Clear Status" is offered only when the book has an explicit status set. + if (book.readingStatus === 'finished' || book.readingStatus === 'unread') { + ids.push('clearStatus'); + } + ids.push('showDetails', 'showInFinder'); + if (book.uploadedAt && !book.downloadedAt) ids.push('download'); + if (!book.uploadedAt && book.downloadedAt) ids.push('upload'); + // Share is offered for any local-or-uploaded book; the dialog uploads first + // if the book hasn't been pushed yet. + if (book.downloadedAt || book.uploadedAt) ids.push('share'); + ids.push('delete'); + return ids; +};