feat: unify navigation handling with system-wide continuous navigation#600
feat: unify navigation handling with system-wide continuous navigation#600daveallie merged 22 commits intocrosspoint-reader:masterfrom
Conversation
|
Testing in device and it's working everywhere I can think to try. The only thing I wasn't sure was whether I'd find that holding the page turn buttons (when long-press chapter switch is turned off) would now flip pages. Not sure that it should, but it did "feel" a bit inconsistent. |
|
@istiak-tridip been testing this, works really well. And simplifies the code too, that's nice. One thing I thought of: on lists that have everything on one screen (pageitems == totalitems) can we make same behavior as in settings - scrolling one by one? That would be useful for cases when chapter list is short, or when file browser have everything on one screen |
|
Excellent suggestion @osteotek. I have made the changes so that when items fit on a single page, they will scroll one by one. |
|
@lukestein, could you explain what you mean by "flip pages"? When reading a book or on the chapter selection page? |
Sorry for lack of clarity. I meant when reading a book. Long-pressing the forward/back buttons could continue to advance to the previous/next page if "Long-press Chapter Skip" is set to OFF. |
|
I've just stumbled upon this one; maybe the idea I described in #615 is not needed if this is merged and it also works when reading a book. |
|
@lukestein I just want to make sure I understand: when |
@istiak-tridip yes thank you. That is exactly what I suggested. But I thought a bit more and I would suggest actually we wait for that in another PR. Some people may not want it as a default. Let's keep your PR behavior as it is. Thank you! After we get this merged and now that we have #451 we can maybe refactor the setting so long-press in reader mode does either
|
lukestein
left a comment
lukestein
left a comment
There was a problem hiding this comment.
No code review completed, but tested for several days on device and seems to work great.
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared ButtonNavigator utility to standardize navigation handling across activities and adds “hold-to-advance” continuous navigation behavior system-wide (including page-based navigation where applicable), while also normalizing wrap-around/index calculations.
Changes:
- Added a new
ButtonNavigatorhelper that centralizes press/release/continuous navigation handling and provides shared index/page-index math. - Updated multiple activities (home, settings, library, WiFi selection, OPDS browser, keyboard, chapter selectors) to use
ButtonNavigatorfor consistent navigation behavior. - Initialized the global
MappedInputManagerhook forButtonNavigatorduring startup.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/ButtonNavigator.h | New navigation helper API + index/page-index utilities. |
| src/util/ButtonNavigator.cpp | New navigation helper implementation (press/release/continuous logic). |
| src/main.cpp | Registers the global MappedInputManager for ButtonNavigator. |
| src/activities/util/KeyboardEntryActivity.h | Adds ButtonNavigator member for keyboard navigation. |
| src/activities/util/KeyboardEntryActivity.cpp | Migrates keyboard navigation to unified press+continuous handling. |
| src/activities/settings/SettingsActivity.h | Adds ButtonNavigator member. |
| src/activities/settings/SettingsActivity.cpp | Replaces per-button navigation with unified next/previous handling. |
| src/activities/settings/KOReaderSettingsActivity.h | Adds ButtonNavigator member. |
| src/activities/settings/KOReaderSettingsActivity.cpp | Migrates submenu navigation to unified next/previous handling. |
| src/activities/settings/CategorySettingsActivity.h | Adds ButtonNavigator member. |
| src/activities/settings/CategorySettingsActivity.cpp | Migrates category settings navigation to unified next/previous handling. |
| src/activities/settings/CalibreSettingsActivity.h | Adds ButtonNavigator member. |
| src/activities/settings/CalibreSettingsActivity.cpp | Migrates Calibre/OPDS settings navigation to unified next/previous handling. |
| src/activities/reader/XtcReaderChapterSelectionActivity.h | Adds ButtonNavigator member for chapter selector. |
| src/activities/reader/XtcReaderChapterSelectionActivity.cpp | Reworks chapter navigation to release-step + continuous page-step behavior. |
| src/activities/reader/EpubReaderChapterSelectionActivity.h | Adds ButtonNavigator member for chapter selector. |
| src/activities/reader/EpubReaderChapterSelectionActivity.cpp | Reworks chapter navigation to release-step + continuous page-step behavior. |
| src/activities/network/WifiSelectionActivity.h | Adds ButtonNavigator member. |
| src/activities/network/WifiSelectionActivity.cpp | Migrates WiFi list navigation to unified next/previous handling. |
| src/activities/network/NetworkModeSelectionActivity.h | Adds ButtonNavigator member. |
| src/activities/network/NetworkModeSelectionActivity.cpp | Migrates network-mode menu navigation to unified next/previous handling. |
| src/activities/home/MyLibraryActivity.h | Adds ButtonNavigator member. |
| src/activities/home/MyLibraryActivity.cpp | Migrates list navigation to release-step + continuous page-step behavior. |
| src/activities/home/HomeActivity.h | Adds ButtonNavigator member. |
| src/activities/home/HomeActivity.cpp | Migrates home menu navigation to unified next/previous handling. |
| src/activities/browser/OpdsBookBrowserActivity.h | Adds ButtonNavigator member. |
| src/activities/browser/OpdsBookBrowserActivity.cpp | Migrates OPDS browsing navigation to release-step + continuous page-step behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class ButtonNavigator final { | ||
| using Callback = std::function<void()>; | ||
| using Buttons = std::vector<MappedInputManager::Button>; | ||
|
|
||
| const uint16_t continuousStartMs; |
There was a problem hiding this comment.
Buttons is defined as std::vector and many call sites pass {...}/getNextButtons() each loop iteration. This can cause repeated heap allocations and fragmentation on the ESP32. Consider switching the API to take a non-owning view (e.g., std::span<const Button> or std::initializer_list<Button>), and make getNextButtons()/getPreviousButtons() return a static std::array/span instead of allocating vectors.
crosspoint-reader#576, crosspoint-reader#587, crosspoint-reader#600 into custom-v16 Integrate 5 upstream PRs: - crosspoint-reader#612: WiFi error text fixes - crosspoint-reader#552: Home cover dimension fix - crosspoint-reader#576: Don't wake on USB connect - crosspoint-reader#587: Boot loop escape - crosspoint-reader#600: System-wide continuous navigation (ButtonNavigator) Manually added ButtonNavigator to EpubReaderMenuActivity (custom file not in upstream) for hold-to-navigate support in reading menu. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Apologies, a few recently merged PRs have generated a bunch of conflicts, we can get this merged once resolved. |
istiak-tridip
dismissed stale reviews from lukestein and osteotek
via
4b09e7c
|
@daveallie I've resolved the merge conflicts. Fingers crossed this gets merged before the v1 release to avoid further merge conflicts 😉 |
istiak-tridip
force-pushed
the
feat/nav
branch
from
798dfb3 to
04f422e
Compare
|
Hi @daveallie, just a gentle follow‑up on this PR when you have a moment. Thanks! |
daveallie
left a comment
daveallie
left a comment
There was a problem hiding this comment.
Sorry for the delay here, the code looks good and it feels great on device
crosspoint-reader#600) This PR unifies navigation handling & adds system-wide support for continuous navigation. ## Summary Holding down a navigation button now continuously advances through items until the button is released. This removes the need for repeated press-and-release actions and makes navigation faster and smoother, especially in long menus or documents. When page-based navigation is available, it will navigate through pages. If not, it will progress through menu items or similar list-based UI elements. Additionally, this PR fixes inconsistencies in wrap-around behavior and navigation index calculations. Places where the navigation system was updated: - Home Page - Settings Pages - My Library Page - WiFi Selection Page - OPDS Browser Page - Keyboard - File Transfer Page - XTC Chapter Selector Page - EPUB Chapter Selector Page I’ve tested this on the device as much as possible and tried to match the existing behavior. Please let me know if I missed anything. Thanks 🙏  --- Following the request from @osteotek and @daveallie for system-wide support, the old PR (crosspoint-reader#379) has been closed in favor of this consolidated, system-wide implementation. --- ### AI Usage Did you use AI tools to help write this code? _**PARTIALLY**_ --------- Co-authored-by: Dave Allie <[email protected]>
…king-space * master: feat: use natural sort in file browser (crosspoint-reader#722) fix: issue if book href are absolute url and not relative to server (crosspoint-reader#741) feat: unify navigation handling with system-wide continuous navigation (crosspoint-reader#600) feat: Add Italian hyphenation support (crosspoint-reader#584) feat: Add percentage support to CSS properties (crosspoint-reader#738) Use GITHUB_REF_NAME over GITHUB_HEAD_REF in release candidate workflow Move release candidate workflow to manual dispatch fix: Allow OTA update from RC build to full release (crosspoint-reader#778) refactor: Rename "Embedded Style" to "Book's Embedded Style" (crosspoint-reader#746) perf: optimize drawPixel() (crosspoint-reader#748) feat: wakeup target detection (crosspoint-reader#731) fix: Scrolling page items calculation (crosspoint-reader#716) feat: optimize fillRectDither (crosspoint-reader#737) fix: increase lyra sideButtonHintsWidth to 30 (crosspoint-reader#727) fix: Remove separations after style changes (crosspoint-reader#720) fix: Lag before displaying covers on home screen (crosspoint-reader#721) feat: Add Settings for toggling CSS on or off (crosspoint-reader#717) Use GITHUB_HEAD_REF release: 1.0.0
* master: (25 commits) fix: Reduce MIN_SIZE_FOR_POPUP to 10KB (crosspoint-reader#809) docs: Update USER_GUIDE.md (crosspoint-reader#817) fix: Prevent sleeping when in OPDS browser / downloading books (crosspoint-reader#818) feat: Extend python debugging monitor functionality (keyword filter / suppress) (crosspoint-reader#810) docs: Update USER_GUIDE.md (crosspoint-reader#808) feat: Connect to last wifi by default (crosspoint-reader#752) feat: use natural sort in file browser (crosspoint-reader#722) fix: issue if book href are absolute url and not relative to server (crosspoint-reader#741) feat: unify navigation handling with system-wide continuous navigation (crosspoint-reader#600) feat: Add Italian hyphenation support (crosspoint-reader#584) feat: Add percentage support to CSS properties (crosspoint-reader#738) Use GITHUB_REF_NAME over GITHUB_HEAD_REF in release candidate workflow Move release candidate workflow to manual dispatch fix: Allow OTA update from RC build to full release (crosspoint-reader#778) refactor: Rename "Embedded Style" to "Book's Embedded Style" (crosspoint-reader#746) perf: optimize drawPixel() (crosspoint-reader#748) feat: wakeup target detection (crosspoint-reader#731) fix: Scrolling page items calculation (crosspoint-reader#716) feat: optimize fillRectDither (crosspoint-reader#737) fix: increase lyra sideButtonHintsWidth to 30 (crosspoint-reader#727) ...
crosspoint-reader#600) This PR unifies navigation handling & adds system-wide support for continuous navigation. ## Summary Holding down a navigation button now continuously advances through items until the button is released. This removes the need for repeated press-and-release actions and makes navigation faster and smoother, especially in long menus or documents. When page-based navigation is available, it will navigate through pages. If not, it will progress through menu items or similar list-based UI elements. Additionally, this PR fixes inconsistencies in wrap-around behavior and navigation index calculations. Places where the navigation system was updated: - Home Page - Settings Pages - My Library Page - WiFi Selection Page - OPDS Browser Page - Keyboard - File Transfer Page - XTC Chapter Selector Page - EPUB Chapter Selector Page I’ve tested this on the device as much as possible and tried to match the existing behavior. Please let me know if I missed anything. Thanks 🙏  --- Following the request from @osteotek and @daveallie for system-wide support, the old PR (crosspoint-reader#379) has been closed in favor of this consolidated, system-wide implementation. --- ### AI Usage Did you use AI tools to help write this code? _**PARTIALLY**_ --------- Co-authored-by: Dave Allie <[email protected]>
|
On this PR - just to check - I noticed that holding down in the settings menu cycles between tabs. Is that intentional? If not, I was thinking the following may be improvements:
And/Or
|
This PR unifies navigation handling & adds system-wide support for continuous navigation.
Summary
Holding down a navigation button now continuously advances through items until the button is released. This removes the need for repeated press-and-release actions and makes navigation faster and smoother, especially in long menus or documents.
When page-based navigation is available, it will navigate through pages. If not, it will progress through menu items or similar list-based UI elements.
Additionally, this PR fixes inconsistencies in wrap-around behavior and navigation index calculations.
Places where the navigation system was updated:
I’ve tested this on the device as much as possible and tried to match the existing behavior. Please let me know if I missed anything. Thanks 🙏
Following the request from @osteotek and @daveallie for system-wide support, the old PR (#379) has been closed in favor of this consolidated, system-wide implementation.
AI Usage
Did you use AI tools to help write this code? PARTIALLY