feat: add two-button navigation to dictionary word select - #2981

Learnt from: zgredex
Repo: crosspoint-reader/crosspoint-reader PR: 1775
File: src/components/themes/lyra/LyraCarouselTheme.cpp:55-72
Timestamp: 2026-04-28T09:44:20.329Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, carousel navigation loop, lines ~386-403), the Up/Down button symmetry — where both Up and Down toggle between the carousel row and the icon menu row — is intentional by design. Do not flag this as a UX inconsistency or suggest differentiated directional behavior in future reviews.

Learnt from: Pankaj-Patil22
Repo: crosspoint-reader/crosspoint-reader PR: 1650
File: src/activities/reader/EpubReaderActivity.cpp:1038-1075
Timestamp: 2026-04-22T16:29:04.799Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/EpubReaderActivity.cpp, EpubReaderActivity::adaptReadingSpeed), setting `lastForwardWasAccidental = true` before the short-page guard is intentional by design. Any forward turn under MIN_ADAPT_ELAPSED_MS (2 s) — regardless of page length — is treated as a potentially accidental tap. A subsequent Back press always cancels it unconditionally (without applying slowdown or incrementing skipForwardAdaptCount), so the short-page guard ordering is irrelevant. Do not flag this ordering as a bug in future reviews.

Learnt from: Pankaj-Patil22
Repo: crosspoint-reader/crosspoint-reader PR: 1650
File: src/activities/reader/EpubReaderActivity.cpp:155-158
Timestamp: 2026-04-22T16:28:42.649Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/EpubReaderActivity.cpp, EpubReaderActivity::loop), the condition `skipForwardAdaptCount == 0 && (millis() - lastPageTurnTime) >= pageTurnDuration` is intentional. After a Smart Back turn, auto-turn is deliberately paused (skipForwardAdaptCount > 0) so the user has full manual control while re-reading the page they returned to. Auto-turn resumes only after the user manually advances through the partial re-read pages (decrementing the counter to 0). Do not flag this as a bug in future reviews.

Learnt from: ariel-lindemann
Repo: crosspoint-reader/crosspoint-reader PR: 1133
File: lib/I18n/translations/finnish.yaml:92-92
Timestamp: 2026-02-23T22:11:42.181Z
Learning: In translation YAML files located in lib/I18n/translations/, literal percent signs (%) in strings (e.g., STR_HIDE_BATTERY, STR_GO_TO_PERCENT, STR_PERCENT_STEP_HINT) do not require escaping as %% and this convention is consistent across all language translations. Ensure that this pattern is followed for all YAML files in this directory to avoid introducing incorrect formatting markers in localized strings.

Learnt from: jpirnay
Repo: crosspoint-reader/crosspoint-reader PR: 1495
File: lib/I18n/translations/german.yaml:294-322
Timestamp: 2026-03-25T10:28:30.516Z
Learning: In crosspoint-reader, it is expected to include “pre-emptive” translation keys in non-English YAML files before the corresponding English keys exist (e.g., german.yaml ahead of en.yaml). When reviewing changes to files under lib/I18n/translations/*.yaml, do not flag missing corresponding English keys in non-English locales as an error if the repo’s gen_i18n.py fallback mechanism is intended to handle missing English keys safely until the pending PRs land.

Learnt from: KymAndriy
Repo: crosspoint-reader/crosspoint-reader PR: 1684
File: lib/I18n/translations/ukrainian.yaml:29-29
Timestamp: 2026-04-17T18:59:20.878Z
Learning: For Ukrainian (and other Slavic-language) translation YAML strings in lib/I18n/translations/*.yaml: if a format string contains a numeric placeholder produced via snprintf-style formatting (e.g., %zu) and runtime noun inflection is not feasible, place the relevant noun in genitive plural before the number in the string (e.g., "мереж %zu") rather than after it (e.g., "мережу %zu"). This avoids incorrect case forms that would otherwise result from the value affecting inflection (e.g., wrong forms like "мережу/мережі/мереж"). In such cases, do not flag the word order as incorrect.

Learnt from: loic-daigle
Repo: crosspoint-reader/crosspoint-reader PR: 2961
File: src/MappedInputManager.h:111-116
Timestamp: 2026-08-10T20:39:38.043Z
Learning: For power-button duration classification, `src/main.cpp` treats a press as long only when `heldTime > CrossPointSettings::getPowerButtonDuration()`. `MappedInputManager::wasShortPowerSelectClick()` intentionally uses `heldTime <= CrossPointSettings::getPowerButtonDuration()` so the classifications are complementary. A duration equal to the threshold is a short press and must produce the configured Select action.

Learnt from: raygan
Repo: crosspoint-reader/crosspoint-reader PR: 1116
File: src/activities/settings/WallabagSettingsActivity.cpp:57-75
Timestamp: 2026-02-23T06:18:08.408Z
Learning: When navigating from a settings activity to KeyboardEntryActivity (e.g., KOReaderSettingsActivity, WallabagSettingsActivity), call exitActivity() on the parent before calling enterNewActivity(new KeyboardEntryActivity(...)). The KeyboardEntryActivity should then call exitActivity() via its callbacks to return to the parent. Note: ConfirmationActivity does not require exitActivity() before entering KeyboardEntryActivity. Apply this pattern to similar settings activities in this module.

Learnt from: ngxson
Repo: crosspoint-reader/crosspoint-reader PR: 1218
File: src/activities/ActivityManager.cpp:254-265
Timestamp: 2026-02-27T22:49:59.600Z
Learning: In this codebase, assertions are always enabled (no NDEBUG). Use assert() to crash on programmer errors and surface logic bugs during development and in production builds. Do not rely on asserts for runtime error handling; they should enforce invariants that must always hold. Keep asserts side-effect free and inexpensive, and avoid relying on them for user-visible failures. Include <cassert> where appropriate and document the invariant being tested.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1245
File: lib/Epub/Epub/Section.cpp:277-308
Timestamp: 2026-03-02T10:14:16.036Z
Learning: Guideline: Strengthen serialization::readString to defend against unbounded growth when reading from disk data. Implement and enforce a maximum allowed length (e.g., a configured or reasonable constant) and validate the incoming length before resizing or allocating. Audit all call sites (e.g., BookMetadataCache, TextBlock, KOReaderCredentialStore, Section cache readers) to ensure they do not rely on unbounded len-based resizing. If the readString API must remain, add internal safeguards (bounds checks, length validation, and error handling) so per-call-site validations are not required. Ensure Section cache files remain versioned (SECTION_FILE_VERSION) and parameter mismatches invalidate caches, but do not rely on unsafe allocations; prefer safe, bounded reads with explicit errors on invalid data.

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 1629
File: src/activities/home/HomeActivity.cpp:119-120
Timestamp: 2026-04-12T12:28:33.205Z
Learning: When reviewing code in this repository (C/C++ sources), set review comment severity according to this policy:
- Use **Major** (🟠) only for defects with realistic risk of crash, out-of-memory (OOM), invalid pointer dereference, data corruption, or other severe issues that are unlikely to be caught in casual/manual device testing.
- Use **Minor** or informational for UX gaps, logic edge-cases, style issues, or missing feature completeness that the author can verify (or has verified) through normal device use.
- Do **not** escalate severity to Major based on behavioral/UX observations alone; assume the author has already tested the feature on their own device and only treat issues as Major if they match the high-risk defect categories above.

Learnt from: jeremydk
Repo: crosspoint-reader/crosspoint-reader PR: 2076
File: src/network/HttpDownloader.cpp:166-171
Timestamp: 2026-05-24T21:10:19.897Z
Learning: Follow the repo’s CLAUDE.md guidance: avoid adding error handling, fallbacks, or extra validation for scenarios that are provably unreachable given existing internal invariants and framework guarantees. Only add validation/guards at true system boundaries (e.g., user input, external APIs/network/IPC). For internal C++ APIs like HttpDownloader, do not add defensive checks against misuse (e.g., an empty std::function callback) when there is no reachable call site that can supply such values—guards in those cases are explicitly discouraged.

Learnt from: itsthisjustin
Repo: crosspoint-reader/crosspoint-reader PR: 2957
File: src/activities/browser/OpdsBookBrowserActivity.cpp:58-64
Timestamp: 2026-08-10T19:01:20.991Z
Learning: In crosspoint-reader activities, navigation heap-allocates activity instances and exitActivity() in src/main.cpp deletes them. Treat each activity instance's onEnter() as running exactly once; per-instance UI actions registered there do not accumulate across navigations, so do not flag them as duplicate registrations unless the lifecycle or ownership model changes.

Learnt from: zgredex
Repo: crosspoint-reader/crosspoint-reader PR: 1775
File: src/components/themes/lyra/LyraCarouselTheme.cpp:55-72
Timestamp: 2026-04-28T09:44:20.329Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/home/HomeActivity.cpp, carousel navigation loop, lines ~386-403), the Up/Down button symmetry — where both Up and Down toggle between the carousel row and the icon menu row — is intentional by design. Do not flag this as a UX inconsistency or suggest differentiated directional behavior in future reviews.

Learnt from: Zyd8
Repo: crosspoint-reader/crosspoint-reader PR: 1576
File: src/activities/home/RecentBooksActivity.cpp:65-90
Timestamp: 2026-04-05T19:21:32.486Z
Learning: In crosspoint-reader/crosspoint-reader (src/util/ButtonNavigator.cpp, shouldSuppressListNavForMidpointChord), the guard-clearing tick intentionally still returns `true` when it clears `s_midpointChordReleaseGuard`. This is correct because all list navigation in the activity uses `onNextRelease`/`onPreviousRelease`, which only fire on a fresh press+release cycle. The clearing tick therefore only swallows the stale release from the chord buttons themselves. Any subsequent Up/Down tap starts a new press+release cycle and will not be suppressed. Do not flag this as a dropped-input bug in future reviews.

Learnt from: aarons
Repo: crosspoint-reader/crosspoint-reader PR: 1659
File: src/activities/reader/ReaderUtils.h:48-51
Timestamp: 2026-04-13T23:10:44.397Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/ReaderUtils.h, detectRotation), the rotation logic uses "content rotation" semantics, not "device rotation" semantics. The bottom button (isBottom=true) rotates the on-screen content clockwise, which physically corresponds to rotating the device counter-clockwise. The enum step `(orientation - 1 + COUNT) % COUNT` achieves CW content rotation; `(orientation + 1) % COUNT` achieves CCW content rotation. This is intentional. Do not flag as reversed. Named helpers `rotateContentCW` / `rotateContentCCW` were suggested by the author as a potential future clarity improvement.

Learnt from: Pankaj-Patil22
Repo: crosspoint-reader/crosspoint-reader PR: 1650
File: src/activities/reader/EpubReaderActivity.cpp:1038-1075
Timestamp: 2026-04-22T16:29:04.799Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/EpubReaderActivity.cpp, EpubReaderActivity::adaptReadingSpeed), setting `lastForwardWasAccidental = true` before the short-page guard is intentional by design. Any forward turn under MIN_ADAPT_ELAPSED_MS (2 s) — regardless of page length — is treated as a potentially accidental tap. A subsequent Back press always cancels it unconditionally (without applying slowdown or incrementing skipForwardAdaptCount), so the short-page guard ordering is irrelevant. Do not flag this ordering as a bug in future reviews.

Learnt from: Pankaj-Patil22
Repo: crosspoint-reader/crosspoint-reader PR: 1650
File: src/activities/reader/EpubReaderActivity.cpp:155-158
Timestamp: 2026-04-22T16:28:42.649Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/EpubReaderActivity.cpp, EpubReaderActivity::loop), the condition `skipForwardAdaptCount == 0 && (millis() - lastPageTurnTime) >= pageTurnDuration` is intentional. After a Smart Back turn, auto-turn is deliberately paused (skipForwardAdaptCount > 0) so the user has full manual control while re-reading the page they returned to. Auto-turn resumes only after the user manually advances through the partial re-read pages (decrementing the counter to 0). Do not flag this as a bug in future reviews.

Learnt from: winst0niuss
Repo: crosspoint-reader/crosspoint-reader PR: 2840
File: src/activities/util/KeyboardEntryActivity.cpp:733-738
Timestamp: 2026-08-02T12:14:47.878Z
Learning: In `src/activities/util/KeyboardEntryActivity.cpp`, `KeyboardEntryActivity::lineBreakEnd()` must return UTF-8 code-point boundaries. Its byte-index binary search can stop inside a multi-byte sequence, so it snaps the result backward to a boundary, remeasures the range, and steps backward by complete code points until it fits. It must retain at least one complete code point to guarantee that the render wrapping loop advances. `KeyboardEntryActivity::loop()` already snaps touch-derived cursor positions backward from UTF-8 continuation bytes before assigning `cursorPos`.

Learnt from: W-Floyd
Repo: crosspoint-reader/crosspoint-reader PR: 0
File: :0-0
Timestamp: 2026-07-24T21:15:54.622Z
Learning: In `src/util/Dictionary.cpp`, a definition read that returns fewer nonnegative bytes than requested intentionally succeeds and resizes the definition to the bytes read. This preserves compatibility with dictionaries whose final index entry extends beyond EOF; do not treat this short-read case as a hard lookup failure without an intentional behavior change.

Learnt from: IjonFryderyk
Repo: crosspoint-reader/crosspoint-reader PR: 2453
File: src/activities/reader/EpubReaderActivity.cpp:354-364
Timestamp: 2026-06-27T20:07:32.056Z
Learning: In `src/activities/reader/EpubReaderFootnotesActivity`, when the activity is entered from `src/activities/reader/EpubReaderActivity.cpp` via the long-press Confirm footnotes flow, Confirm may still be held on entry. `EpubReaderFootnotesActivity::onEnter()` should detect a held select button (Confirm or Power) and use a one-shot `ignoreFirstSelectRelease` latch so the opening release is swallowed and does not immediately select footnote index 0. Other entry paths such as the reader menu and short power-button footnotes path do not need this latch because no select button is held on entry.

Learnt from: Zyd8
Repo: crosspoint-reader/crosspoint-reader PR: 1576
File: src/util/ButtonNavigator.h:43-47
Timestamp: 2026-04-05T19:31:31.692Z
Learning: In crosspoint-reader/crosspoint-reader (src/util/ButtonNavigator.cpp), `clearMidpointChordReleaseGuard()` intentionally sets `s_suppressUntilChordRelease` in addition to resetting the post-release guard. This blocks `beginUpDownChord()` from firing on the first loop tick of a new activity if the midpoint chord was still physically held during the `Activity::onEnter()` transition. The fix prevents cross-screen midpoint chord leaks without requiring each activity's `onEnter()` to call `isMidpointChordHeld()`. Do not flag this dual-write as redundant in future reviews.

Learnt from: aarons
Repo: crosspoint-reader/crosspoint-reader PR: 1659
File: src/activities/reader/ReaderUtils.h:48-51
Timestamp: 2026-04-13T22:45:24.939Z
Learning: In crosspoint-reader/crosspoint-reader (src/activities/reader/ReaderUtils.h, detectRotation), the rotation direction logic `isBottom ? (orientation - 1 + COUNT) % COUNT : (orientation + 1) % COUNT` is intentionally correct as written. Despite the enum ordering PORTRAIT→LANDSCAPE_CW→INVERTED→LANDSCAPE_CCW suggesting "+1 = clockwise", on the actual device hardware the bottom button (isBottom=true) stepping backward through the enum (-1) produces a clockwise rotation and the top button (+1) produces counter-clockwise. Do not flag this as reversed in future reviews.

Learnt from: pablohc
Repo: crosspoint-reader/crosspoint-reader PR: 1534
File: src/activities/boot_sleep/SleepActivity.cpp:0-0
Timestamp: 2026-04-05T10:43:16.171Z
Learning: In crosspoint-reader/crosspoint-reader (src/CrossPointSettings.h), CrossPointSettings::SLEEP_COVER_OVERLAY enum has values: OVERLAY_OFF=0, OVERLAY_WHITE=1, OVERLAY_GRAY=2, OVERLAY_BLACK=3 (as of commit 2471b5a in PR `#1534`). The default sleepCoverOverlay byte is 0 (OVERLAY_OFF). Do not use the old ordering WHITE=0/GRAY=1/BLACK=2/OFF=3 in future reviews.