From e74e52b23a0729e65bab697dca8f6c968c5e1e98 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Fri, 17 Apr 2026 20:59:49 +0200 Subject: [PATCH] Review comments --- src/activities/ActivityManager.cpp | 10 +++++-- src/activities/ActivityManager.h | 26 ++++++++++++------- src/activities/home/FileBrowserActivity.cpp | 2 +- .../home/GlobalBookmarksActivity.cpp | 25 ++++++++++++++++-- src/activities/home/GlobalBookmarksActivity.h | 5 ++-- 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/activities/ActivityManager.cpp b/src/activities/ActivityManager.cpp index 92036049..c2490a8a 100644 --- a/src/activities/ActivityManager.cpp +++ b/src/activities/ActivityManager.cpp @@ -260,8 +260,11 @@ void ActivityManager::goToRecentBooks(int focusIndex) { replaceActivity(std::make_unique(renderer, mappedInput, focusIndex)); } -void ActivityManager::goToGlobalBookmarks() { - replaceActivity(std::make_unique(renderer, mappedInput)); +void ActivityManager::goToGlobalBookmarks() { goToGlobalBookmarks({}); } + +void ActivityManager::goToGlobalBookmarks(ReturnHint hint) { + hasReturnHint = false; + replaceActivity(std::make_unique(renderer, mappedInput, std::move(hint))); } void ActivityManager::goToBrowser() { @@ -319,6 +322,9 @@ void ActivityManager::returnFromChild() { case ReturnTo::RecentBooks: goToRecentBooks(hint.selectIndex); break; + case ReturnTo::GlobalBookmarks: + goToGlobalBookmarks(std::move(hint)); + break; case ReturnTo::Home: default: goHome(std::move(hint.selectName), hint.selectIndex); diff --git a/src/activities/ActivityManager.h b/src/activities/ActivityManager.h index f0e6ec28..5ab9d04f 100644 --- a/src/activities/ActivityManager.h +++ b/src/activities/ActivityManager.h @@ -17,17 +17,19 @@ class RenderLock; // forward declaration // Where a "child" activity (launched via one of the replaceWith* helpers) should route // control when it exits successfully. See ActivityManager::returnFromChild(). -enum class ReturnTo : uint8_t { Home, FileBrowser, RecentBooks }; +enum class ReturnTo : uint8_t { Home, FileBrowser, RecentBooks, GlobalBookmarks }; // Minimal state the returning parent needs to restore its previous view (directory, -// focused item, list index). Kept as a plain struct stored by value on the -// ActivityManager — single instance, overwritten per transition, no heap churn -// beyond the two small std::strings. +// focused item, list index, or bookmark selection). Kept as a plain struct stored by +// value on the ActivityManager — single instance, overwritten per transition, no heap +// churn beyond the small strings. struct ReturnHint { ReturnTo target = ReturnTo::Home; - std::string path; // FileBrowser directory to restore - std::string selectName; // item to re-focus in a list (file name, book title) - int selectIndex = -1; // e.g. Recents index + std::string path; // FileBrowser directory to restore + std::string selectName; // item to re-focus in a list (file name, book title) + int selectIndex = -1; // e.g. Recents index + std::string selectionContext; // optional activity-specific restore key + int selectBookmarkIndex = -1; // optional bookmark index for GlobalBookmarks }; /** @@ -83,9 +85,12 @@ class ActivityManager { // into the next one. bool drainInput = false; - // Where returnFromChild() should route to. Set by replaceWith*() helpers, cleared - // in returnFromChild(). Cleared on any manual goHome()/goTo*() to avoid stale hints - // outliving the flow they were recorded for. + // Where returnFromChild() should route to. Set by replaceWith*() helpers and + // preserved across plain goTo*() chains so a chained navigation flow can still + // restore its original parent state. Cleared only by returnFromChild() or by + // explicit goHome()/replaceWith*() calls, not by ordinary goTo*() transitions. + // Relevant symbols: ReturnHint, returnHint, hasReturnHint, returnFromChild(), + // goHome(), goTo*(), replaceWith*(). ReturnHint returnHint; bool hasReturnHint = false; @@ -109,6 +114,7 @@ class ActivityManager { void goToFileBrowser(std::string path = {}, std::string focusName = {}); void goToRecentBooks(int focusIndex = -1); void goToGlobalBookmarks(); + void goToGlobalBookmarks(ReturnHint hint); void goToBrowser(); void goToReader(std::string path); void goToKOReaderSync(); diff --git a/src/activities/home/FileBrowserActivity.cpp b/src/activities/home/FileBrowserActivity.cpp index 0e5229a9..d3c07f66 100644 --- a/src/activities/home/FileBrowserActivity.cpp +++ b/src/activities/home/FileBrowserActivity.cpp @@ -323,5 +323,5 @@ void FileBrowserActivity::render(RenderLock&&) { size_t FileBrowserActivity::findEntry(const std::string& name) const { for (size_t i = 0; i < files.size(); i++) if (files[i] == name) return i; - return 0; + return files.size(); } \ No newline at end of file diff --git a/src/activities/home/GlobalBookmarksActivity.cpp b/src/activities/home/GlobalBookmarksActivity.cpp index 0739f8de..5e50c6c9 100644 --- a/src/activities/home/GlobalBookmarksActivity.cpp +++ b/src/activities/home/GlobalBookmarksActivity.cpp @@ -27,6 +27,26 @@ void GlobalBookmarksActivity::onEnter() { const int first = firstSelectableIndex(); selectorIndex = first >= 0 ? first : 0; + if (restoreHint.target == ReturnTo::GlobalBookmarks) { + const auto& entries = GLOBAL_BOOKMARKS.getEntries(); + if (!restoreHint.selectionContext.empty() && restoreHint.selectBookmarkIndex >= 0) { + for (size_t i = 0; i < rows.size(); ++i) { + const auto& row = rows[i]; + if (row.isSeparator) continue; + if (row.bookmarkIndex == static_cast(restoreHint.selectBookmarkIndex) && + row.bookIndex < entries.size() && entries[row.bookIndex].sourcePath == restoreHint.selectionContext) { + selectorIndex = static_cast(i); + break; + } + } + } + if (selectorIndex < 0 || selectorIndex >= static_cast(rows.size()) || isSeparatorRow(selectorIndex)) { + const int fallback = firstSelectableIndex(); + selectorIndex = fallback >= 0 ? fallback : 0; + } + restoreHint = {}; + } + const auto total = static_cast(rows.size()); buttonNavigator.setSelectablePredicate([this](int index) { return !isSeparatorRow(index); }, total); @@ -126,8 +146,9 @@ void GlobalBookmarksActivity::openSelected() { LOG_DBG("GBA", "Jumping to bookmark in %s at %u/%u", entry.sourcePath.c_str(), bm.spineIndex, bm.pageNumber); ReturnHint hint; - hint.target = ReturnTo::Home; - hint.selectName = entry.sourcePath; + hint.target = ReturnTo::GlobalBookmarks; + hint.selectionContext = entry.sourcePath; + hint.selectBookmarkIndex = static_cast(row.bookmarkIndex); activityManager.replaceWithReader(entry.sourcePath, std::move(hint)); } diff --git a/src/activities/home/GlobalBookmarksActivity.h b/src/activities/home/GlobalBookmarksActivity.h index 3f4c9f75..77e505c6 100644 --- a/src/activities/home/GlobalBookmarksActivity.h +++ b/src/activities/home/GlobalBookmarksActivity.h @@ -20,8 +20,8 @@ struct Rect; // header separator or a bookmark entry belonging to the preceding header. class GlobalBookmarksActivity final : public Activity { public: - explicit GlobalBookmarksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput) - : Activity("GlobalBookmarks", renderer, mappedInput) {} + explicit GlobalBookmarksActivity(GfxRenderer& renderer, MappedInputManager& mappedInput, ReturnHint restoreHint = {}) + : Activity("GlobalBookmarks", renderer, mappedInput), restoreHint(std::move(restoreHint)) {} void onEnter() override; void onExit() override; @@ -38,6 +38,7 @@ class GlobalBookmarksActivity final : public Activity { ButtonNavigator buttonNavigator; std::vector rows; int selectorIndex = 0; + ReturnHint restoreHint; void rebuildRows(); std::string getRowTitle(int index) const;