fix: continue reading card classic theme by pablohc · Pull Request #990 · crosspoint-reader/crosspoint-reader

pablohc · 2026-02-19T13:24:30Z

Summary

What is the goal of this PR?
What changes are included?

Adapt card width to cover image aspect ratio in Classic theme
Increase homeTopPadding from 20px to 40px to avoid overlap with battery icon
Card width now calculated from BMP dimensions instead of fixed 240px
Maximum card width limited to 90% of screen width
Falls back to original behavior (half screen width) when no cover available

Additional Context

Solve conflicts in PR fix: dynamic continue reading card sizing based on cover image #683

Before:

PR:

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? < YES >

coderabbitai · 2026-02-19T13:24:47Z

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40f9a93 and d43a775.

📒 Files selected for processing (2)

src/components/themes/BaseTheme.cpp
src/components/themes/BaseTheme.h

🚧 Files skipped from review as they are similar to previous changes (1)

src/components/themes/BaseTheme.h

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-02-19T12:17:00.962Z

Learnt from: Uri-Tauber
Repo: crosspoint-reader/crosspoint-reader PR: 988
File: lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp:649-661
Timestamp: 2026-02-19T12:17:00.962Z
Learning: In ChapterHtmlSlimParser.cpp, when computing footnote word indices in endElement() for footnote links, the wordIndex must be cumulative across the 750-word mid-paragraph flush boundary. The correct calculation is: `wordIndex = wordsExtractedInBlock + currentTextBlock->size()`, not just `currentTextBlock->size()`. This ensures footnotes attach to the page containing their actual anchor word, even after layoutAndExtractLines(false) has extracted and removed earlier words from the block.

Applied to files:

src/components/themes/BaseTheme.cpp

🧬 Code graph analysis (1)

src/components/themes/BaseTheme.cpp (1)

src/components/UITheme.cpp (2)

getCoverThumbPath (69-75)

getCoverThumbPath (69-69)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: cppcheck

🔇 Additional comments (4)

src/components/themes/BaseTheme.cpp (4)

347-394: LGTM: cover-aware width calculation with sensible fallback and clamping.

The aspect-ratio sizing, max-width clamp, and half-width fallback look solid.

417-419: LGTM: bitmap rendering now matches the computed card dimensions.

Clear and consistent with the new sizing logic.

601-602: LGTM: title/author box centering now respects rect.x.

This aligns the overlay with the card geometry.

644-645: LGTM: “Continue Reading” box alignment matches the card center.

Consistent with the updated layout.

📝 Walkthrough

Walkthrough

Compute recent-book card width from cover BMP aspect ratio (when present, capped at 90% of screen); otherwise use half-screen width. Card height remains fixed. Bitmap-based rendering path added (bookmark ribbon suppressed when cover shown). homeTopPadding increased from 20 to 40.

Changes

Cohort / File(s)	Summary
Book Card Rendering & Theme Metrics `src/components/themes/BaseTheme.cpp`, `src/components/themes/BaseTheme.h`	Refactored recent-book rendering to derive card width from BMP cover dimensions when available (aspect-ratio based, clamped to 90% screen); fallback to half-screen width. Added hasCoverImage/BMP render path, adjusted border/selection/bookmark logic and text/layout positioning. Increased `BaseMetrics.values.homeTopPadding` from 20 to 40.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: continue reading card classic theme' accurately describes the main change—adapting the continue reading card in the Classic theme to use cover-aware sizing and increasing padding.
Description check	✅ Passed	The description is directly related to the changeset, clearly explaining the goal (adapt card width to cover image aspect ratio and increase homeTopPadding), listing specific changes, and providing before/after screenshots demonstrating the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/components/themes/BaseTheme.cpp (1)

355-384: Redundant file I/O: BMP opened twice in same render cycle.

The cover BMP file is opened and parsed here (lines 360-383) to get dimensions, then opened and parsed again later (lines 411-437) to render. On embedded devices with SD card I/O, this duplication is costly.

Consider keeping the Bitmap object or file handle alive and reusing it for both dimension calculation and rendering.

♻️ Sketch of refactored approach

+  FsFile coverFile;
+  Bitmap* coverBitmap = nullptr;
+
   if (hasContinueReading && !recentBooks[0].coverBmpPath.empty()) {
     const std::string coverBmpPath =
         UITheme::getCoverThumbPath(recentBooks[0].coverBmpPath, BaseMetrics::values.homeCoverHeight);
 
-    FsFile file;
-    if (Storage.openFileForRead("HOME", coverBmpPath, file)) {
-      Bitmap bitmap(file);
-      if (bitmap.parseHeaders() == BmpReaderError::Ok) {
+    if (Storage.openFileForRead("HOME", coverBmpPath, coverFile)) {
+      coverBitmap = new Bitmap(coverFile);
+      if (coverBitmap->parseHeaders() == BmpReaderError::Ok) {
         hasCoverImage = true;
-        const int imgWidth = bitmap.getWidth();
-        const int imgHeight = bitmap.getHeight();
+        const int imgWidth = coverBitmap->getWidth();
+        const int imgHeight = coverBitmap->getHeight();
         // ... calculate bookWidth ...
+      } else {
+        delete coverBitmap;
+        coverBitmap = nullptr;
+        coverFile.close();
       }
-      file.close();
+      // Keep file open for later rendering
     }
   }

Then reuse coverBitmap in the rendering block (lines 411-437) instead of re-opening.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/themes/BaseTheme.cpp` around lines 355 - 384, The code
currently opens and parses the same cover BMP twice; change the flow to open the
FsFile once, construct a Bitmap (e.g., coverBitmap) and call
parseHeaders()/getWidth()/getHeight() to compute bookWidth, then keep that
Bitmap (and its open FsFile) alive and reuse it in the later rendering block
instead of calling Storage.openFileForRead/Bitmap again; specifically, replace
the local Bitmap/file in the first branch with a persisting local coverBitmap
and coverFile variables visible to both the dimension-calculation branch and the
rendering branch (close the file and destroy the Bitmap only after rendering or
on all fallbacks), preserving the current fallback logic when parseHeaders() !=
BmpReaderError::Ok or imgHeight == 0.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 103fac2 and c6f8da2.

📒 Files selected for processing (2)

src/components/themes/BaseTheme.cpp
src/components/themes/BaseTheme.h

🧰 Additional context used

🧬 Code graph analysis (1)

src/components/themes/BaseTheme.cpp (5)

src/components/UITheme.cpp (2)

getCoverThumbPath (69-75)

getCoverThumbPath (69-69)

src/components/themes/lyra/Lyra3CoversTheme.cpp (1)

bitmap (49-49)

src/components/themes/lyra/LyraTheme.cpp (1)

bitmap (439-439)

src/activities/boot_sleep/SleepActivity.cpp (4)

bitmap (58-58)

bitmap (82-82)

bitmap (97-97)

bitmap (266-266)

src/main.cpp (1)

renderer (39-39)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: cppcheck

🔇 Additional comments (2)

src/components/themes/BaseTheme.h (1)

85-85: LGTM!

The padding increase from 20 to 40 aligns with the PR objective to avoid overlap with the battery icon.

src/components/themes/BaseTheme.cpp (1)

417-427: LGTM!

The rendering logic correctly uses the pre-calculated aspect-ratio-based dimensions and properly handles buffer storage for subsequent navigation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/themes/BaseTheme.cpp`:
- Around line 369-380: The code computes bookWidth in BaseTheme.cpp using
imgHeight but never validates imgWidth, so a zero or negative imgWidth can
produce bookWidth == 0 and break layout; update the logic in the block around
imgHeight/imgWidth/bookWidth to validate both imgWidth > 0 and imgHeight > 0 (or
treat invalid dimensions as fallback), compute aspectRatio only when both are
positive, and otherwise set bookWidth to the fallback (rect.width / 2);
additionally keep the existing maxWidth clamping using baseHeight and rect.width
to ensure the final bookWidth is bounded.

---

Nitpick comments:
In `@src/components/themes/BaseTheme.cpp`:
- Around line 355-384: The code currently opens and parses the same cover BMP
twice; change the flow to open the FsFile once, construct a Bitmap (e.g.,
coverBitmap) and call parseHeaders()/getWidth()/getHeight() to compute
bookWidth, then keep that Bitmap (and its open FsFile) alive and reuse it in the
later rendering block instead of calling Storage.openFileForRead/Bitmap again;
specifically, replace the local Bitmap/file in the first branch with a
persisting local coverBitmap and coverFile variables visible to both the
dimension-calculation branch and the rendering branch (close the file and
destroy the Bitmap only after rendering or on all fallbacks), preserving the
current fallback logic when parseHeaders() != BmpReaderError::Ok or imgHeight ==
0.

src/components/themes/BaseTheme.cpp

pablohc · 2026-02-19T13:29:09Z

Hi @lukestein @osteotek @jonasdiemer, could you please check if everything is working correctly?

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6f8da2 and 62b805b.

📒 Files selected for processing (1)

src/components/themes/BaseTheme.cpp

🧰 Additional context used

🧬 Code graph analysis (1)

src/components/themes/BaseTheme.cpp (4)

src/components/UITheme.cpp (2)

getCoverThumbPath (69-75)

getCoverThumbPath (69-69)

src/components/themes/lyra/Lyra3CoversTheme.cpp (1)

bitmap (49-49)

src/components/themes/lyra/LyraTheme.cpp (1)

bitmap (439-439)

src/activities/boot_sleep/SleepActivity.cpp (4)

bitmap (58-58)

bitmap (82-82)

bitmap (97-97)

bitmap (266-266)

🔇 Additional comments (1)

src/components/themes/BaseTheme.cpp (1)

417-418: LGTM: cover rendering uses computed dimensions directly.

This simplifies positioning and keeps sizing consistent with the new aspect‑ratio calculation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/themes/BaseTheme.cpp`:
- Around line 347-394: The centering math for the top "book" card uses
rect.width only and ignores rect.x (bookX calculation in BaseTheme.cpp), so
update the calculation to center relative to rect.x by adding rect.x: compute
bookX as rect.x plus half the remaining space ((rect.width - bookWidth) / 2).
Also update any subsequent dependent positions that rely on bookX (e.g.,
bookmarkX) to use the new bookX value so all offsets remain correct when rect.x
!= 0.

src/components/themes/BaseTheme.cpp

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62b805b and 40f9a93.

📒 Files selected for processing (2)

src/components/themes/BaseTheme.cpp
src/components/themes/BaseTheme.h

🚧 Files skipped from review as they are similar to previous changes (1)

src/components/themes/BaseTheme.h

🧰 Additional context used

🧬 Code graph analysis (1)

src/components/themes/BaseTheme.cpp (3)

src/components/UITheme.cpp (2)

getCoverThumbPath (69-75)

getCoverThumbPath (69-69)

src/components/themes/lyra/LyraTheme.cpp (1)

bitmap (439-439)

src/activities/boot_sleep/SleepActivity.cpp (4)

bitmap (58-58)

bitmap (82-82)

bitmap (97-97)

bitmap (266-266)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: cppcheck

🔇 Additional comments (1)

src/components/themes/BaseTheme.cpp (1)

417-418: Cover rendering uses computed dimensions — looks good.

Using bookWidth/bookHeight for drawBitmap aligns with the new sizing logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/themes/BaseTheme.cpp`:
- Around line 391-392: The title/author overlay and the "Continue Reading"
overlay are still centered using (rect.width - boxWidth) / 2 which ignores
rect.x; update their X calculations to mirror the book card centering (use
rect.x + (rect.width - boxWidth) / 2) so they align with bookX; locate the code
that computes those overlay X positions (the title/author box calculation and
the "Continue Reading" box calculation) and replace the standalone (rect.width -
boxWidth) / 2 expressions with rect.x + (rect.width - boxWidth) / 2 so overlays
track the card when rect.x is non-zero.

src/components/themes/BaseTheme.cpp

- Adapt card width to cover image aspect ratio in Classic theme - Increase homeTopPadding from 20px to 40px to avoid overlap with battery icon - Card width now calculated from BMP dimensions instead of fixed 240px - Maximum card width limited to 90% of screen width - Falls back to original behavior (half screen width) when no cover available

pablohc · 2026-02-19T23:48:05Z

Hi @daveallie, would it be possible to add it to 1.1.0 RC to fix the classic theme?

daveallie · 2026-02-20T05:19:01Z

Yep, will squeeze it in there

## Summary * **What is the goal of this PR?** * **What changes are included?** - Adapt card width to cover image aspect ratio in Classic theme - Increase homeTopPadding from 20px to 40px to avoid overlap with battery icon - Card width now calculated from BMP dimensions instead of fixed 240px - Maximum card width limited to 90% of screen width - Falls back to original behavior (half screen width) when no cover available ## Additional Context * Solve conflicts in PR #683 Before: <img width="1052" height="1014" alt="image" src="https://github.com/user-attachments/assets/6c857913-d697-4e9e-9695-443c0a4c0804" /> PR: ![Screenshot_2026-02-19-14-22-36-68_99c04817c0de5652397fc8b56c3b3817](https://github.com/user-attachments/assets/81505728-d42e-41bd-bd77-44848e05b1eb) --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< YES >**_

## Summary * **What is the goal of this PR?** * **What changes are included?** - Adapt card width to cover image aspect ratio in Classic theme - Increase homeTopPadding from 20px to 40px to avoid overlap with battery icon - Card width now calculated from BMP dimensions instead of fixed 240px - Maximum card width limited to 90% of screen width - Falls back to original behavior (half screen width) when no cover available ## Additional Context * Solve conflicts in PR crosspoint-reader#683 Before: <img width="1052" height="1014" alt="image" src="https://github.com/user-attachments/assets/6c857913-d697-4e9e-9695-443c0a4c0804" /> PR: ![Screenshot_2026-02-19-14-22-36-68_99c04817c0de5652397fc8b56c3b3817](https://github.com/user-attachments/assets/81505728-d42e-41bd-bd77-44848e05b1eb) --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< YES >**_

coderabbitai bot reviewed Feb 19, 2026

View reviewed changes

src/components/themes/BaseTheme.cpp Outdated Show resolved Hide resolved

pablohc mentioned this pull request Feb 19, 2026

fix: dynamic continue reading card sizing based on cover image #683

Closed

coderabbitai bot reviewed Feb 19, 2026

View reviewed changes

src/components/themes/BaseTheme.cpp Show resolved Hide resolved

pablohc force-pushed the fix/classic-theme-continue-reading-card branch from 62b805b to 40f9a93 Compare February 19, 2026 13:45

coderabbitai bot reviewed Feb 19, 2026

View reviewed changes

src/components/themes/BaseTheme.cpp Show resolved Hide resolved

pablohc force-pushed the fix/classic-theme-continue-reading-card branch from 40f9a93 to d43a775 Compare February 19, 2026 13:59

daveallie approved these changes Feb 20, 2026

View reviewed changes

daveallie merged commit c9faf2a into crosspoint-reader:master Feb 20, 2026
6 checks passed

daveallie mentioned this pull request Feb 22, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

coderabbitai bot mentioned this pull request Feb 23, 2026

feat: wrapped text in GfxRender, implemented in themes so far #1141

Merged

pablohc deleted the fix/classic-theme-continue-reading-card branch February 27, 2026 15:05

coderabbitai bot mentioned this pull request Mar 1, 2026

feat: Recognize and use series metadata #1259

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: continue reading card classic theme#990

fix: continue reading card classic theme#990
daveallie merged 1 commit intocrosspoint-reader:masterfrom
pablohc:fix/classic-theme-continue-reading-card

pablohc commented Feb 19, 2026

Uh oh!

coderabbitai bot commented Feb 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

pablohc commented Feb 19, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

pablohc commented Feb 19, 2026

Uh oh!

daveallie commented Feb 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pablohc commented Feb 19, 2026

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pablohc commented Feb 19, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pablohc commented Feb 19, 2026

Uh oh!

daveallie commented Feb 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Feb 19, 2026 •

edited

Loading