fix: harden EPUB optimiser UI gating, size reporting, and picker teardown by zgredex · Pull Request #1947 · crosspoint-reader/crosspoint-reader

zgredex · 2026-05-11T15:45:13Z

Summary

Four fixes in the web file-upload modal's EPUB optimiser (src/network/html/FilesPage.html):

"Optimize EPUB" was offered for any file type. The convert-options panel was shown unconditionally on every file selection. It now appears only when at least one selected file ends in .epub. When the selection changes back to non-EPUB, the checkbox is force-unchecked via toggleConvertOptions() so the "Optimize & Upload" button label and image-picker state reset cleanly. Mixed selections still show the option — the existing batch loop already filters EPUBs.
Per-file conversion summary reported wrong sizes. logSummary() was being fed totalImageSize / totalNewSize (sum of image bytes only), so whenever the EPUB contained any images, Original size, Optimized size, and Saved (X%) didn't match the actual file on disk. It now receives originalSize = file.size and newSize = newBlob.size. The now-unused accumulators are removed.
Batch summary had no aggregate size totals. batchStats.totalOriginalSize / totalNewSize were declared but never incremented or rendered. They are now wired up: saveToFileBatchLog takes optional (originalSize, newSize), snapshotted before the converted blob replaces the picked file, and the batch summary renders Total original, Total optimised, Total saved (X%) whenever at least one file successfully converted. Files that fell back to original upload (or never converted) contribute 0/0 and don't poison the totals.
Image picker stayed visible after switching to multi-file selection. validateFile() opened the picker on a single-EPUB selection with Advanced Mode expanded, but had no else branch — re-picking multiple files left the previous file's thumbnails and picker-mode layout visible. Added an else that calls the existing idempotent clearImagePicker() so the grid, layout classes, and the Start Conversion button tear down whenever the selection no longer matches the open-picker condition.

Per-image before/after numbers logged by logImage() are unchanged. Reuses the existing formatBytes() helper and .saved / .increased CSS classes — no new styling.

Commits

b38e0561 fix: gate EPUB optimiser by extension and report real file sizes
fef6b242 feat: aggregate Total original / optimised / saved rows in batch summary
4ca511bc fix: clear stale image picker when selection switches to multi-file

Test plan

- Show the "Optimize EPUB" panel only when at least one selected file has the .epub extension; force the checkbox off when the selection changes to non-EPUB so the "Optimize & Upload" button doesn't linger. - Pass the real file/blob sizes to logSummary() instead of the sum-of-image-bytes accumulators, so the conversion summary's Original / Optimized / Saved rows match what's actually on disk.

coderabbitai · 2026-05-11T15:47:41Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 46c4cf87-d7e8-4b61-8c9a-23f0a6dfd2e2

📥 Commits

Reviewing files that changed from the base of the PR and between fef6b24 and 4ca511b.

📒 Files selected for processing (1)

src/network/html/FilesPage.html

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

src/network/html/FilesPage.html

📜 Recent review details

⏰ 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). (1)

GitHub Check: build

📝 Walkthrough

Walkthrough

FilesPage.html: show EPUB conversion options only when at least one selected file is an EPUB; reset conversion/image UI state for non-EPUB or empty selections. Track per-file original/new byte sizes through upload/conversion and include aggregated totals in batch summary only when applicable. convertEpubFile() no longer accumulates image-size totals and calls logSummary(originalSize, newSize, timeElapsed) directly.

Changes

EPUB Conversion Workflow Updates

Layer / File(s)	Summary
Conversion UI State Control `src/network/html/FilesPage.html`	`validateFile()` displays `convertOptions` only when selection includes an `.epub`; hides it otherwise, clears `convertBeforeUpload` via `toggleConvertOptions()`, and resets the image picker when no files are selected.
Batch Log Size Tracking `src/network/html/FilesPage.html`	`saveToFileBatchLog()` now accepts `originalSize`/`newSize` and updates `batchStats.totalOriginalSize`/`totalNewSize`; `finalizeBatchLog()` computes total original/optimised/saved rows and includes them only when `batchStats.totalOriginalSize > 0`; table insertion of size rows is conditional.
Conversion Logging Simplification `src/network/html/FilesPage.html`	`convertEpubFile()` removes `totalImageSize`/`totalNewSize` accumulators and incremental accumulation, and calls `logSummary(originalSize, newSize, timeElapsed)` directly at completion.
Upload: capture converted sizes `src/network/html/FilesPage.html`	`uploadFile()` adds `convOriginalSize`/`convNewSize`, snapshots `file.size` before swapping in converted blob, stores pre/post-conversion sizes after successful conversion, and passes sizes to `saveToFileBatchLog()` on batch complete or `(0,0)` on error.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

crosspoint-reader/crosspoint-reader#1224: Overlapping changes to FilesPage.html's validateFile() and convertEpubFile() for EPUB conversion UI and logging.

Suggested reviewers

martinbrook
znelson

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main changes: EPUB optimizer UI gating by file extension, size reporting fixes, and image picker cleanup.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing all four fixes with concrete examples and a detailed test plan.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai · 2026-05-11T15:50:28Z

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

zgredex · 2026-05-11T15:57:17Z

@coderabbitai full review

coderabbitai · 2026-05-11T15:57:26Z

✅ Actions performed

Full review triggered.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/network/html/FilesPage.html (1)

2950-2956: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear stale image picker when selection is no longer single-EPUB eligible.

If the picker is already visible (from a previous single-EPUB selection), switching to multi-file selection can leave stale picker content on screen. Add an explicit clear when the single-EPUB condition is not met.

💡 Suggested patch

-      if (advancedContent.classList.contains('visible') && files.length === 1 && convertEnabled && files[0].name.toLowerCase().endsWith('.epub')) {
-        showImagePicker(files[0]).catch(err => console.error('Image picker error:', err));
-      }
+      if (advancedContent.classList.contains('visible') && files.length === 1 && convertEnabled && files[0].name.toLowerCase().endsWith('.epub')) {
+        showImagePicker(files[0]).catch(err => console.error('Image picker error:', err));
+      } else {
+        clearImagePicker();
+      }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/network/html/FilesPage.html` around lines 2950 - 2956, The current logic
only calls showImagePicker(files[0]) when the single-EPUB condition is met, but
does not clear an existing image picker when the user switches to a multi-file
or non-EPUB selection; add an explicit clear/hide step in the else path so stale
UI is removed. Specifically, after computing advancedContent, convertEnabled and
checking the condition that leads to showImagePicker(files[0]), add an else
branch that calls a clear/hide routine (e.g., hideImagePicker() or remove the
DOM node with id 'imagePicker' / clearImagePicker()) to remove any existing
picker and cancel any related state; reference showImagePicker,
advancedSettingsContent, convertBeforeUpload and files when locating where to
insert the else/clear logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/network/html/FilesPage.html`:
- Around line 2950-2956: The current logic only calls showImagePicker(files[0])
when the single-EPUB condition is met, but does not clear an existing image
picker when the user switches to a multi-file or non-EPUB selection; add an
explicit clear/hide step in the else path so stale UI is removed. Specifically,
after computing advancedContent, convertEnabled and checking the condition that
leads to showImagePicker(files[0]), add an else branch that calls a clear/hide
routine (e.g., hideImagePicker() or remove the DOM node with id 'imagePicker' /
clearImagePicker()) to remove any existing picker and cancel any related state;
reference showImagePicker, advancedSettingsContent, convertBeforeUpload and
files when locating where to insert the else/clear logic.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d6d3fca-73f2-4045-b0d6-92b198cd4f66

📥 Commits

Reviewing files that changed from the base of the PR and between accd50b and b38e056.

📒 Files selected for processing (1)

src/network/html/FilesPage.html

📜 Review details

🔇 Additional comments (2)

src/network/html/FilesPage.html (2)

2934-2947: Good EPUB-only gating and reset behavior.

Line 2934–Line 2947 correctly restricts Optimize UI to selections containing at least one .epub, and the forced uncheck/reset prevents stale “Optimize & Upload” state.

4513-4513: Nice fix for summary size accuracy.

Line 4513 now reports true file-level sizes (file.size vs newBlob.size), which matches user-visible expectations for “Original/Optimized/Saved”.

Wire up the previously-unused batchStats.totalOriginalSize and totalNewSize: snapshot the picked-file size before the converted blob replaces it, capture convertedBlob.size on success, and pass both through saveToFileBatchLog (now (fileName, succeeded, originalSize=0, newSize=0)). Failed conversions and fallback-to- original uploads pass 0/0 so they don't poison the totals. finalizeBatchLog renders three new rows (Total original / optimised / saved) only when at least one file actually contributed sizes, reusing formatBytes and the existing .saved / .increased styling.

zgredex · 2026-05-11T16:20:45Z

@coderabbitai review

coderabbitai · 2026-05-11T16:20:56Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

mcrosson · 2026-05-11T16:22:46Z

I appreciate the nod towards my ability to review a change like this but I think it best to defer to @itsthisjustin or another whos got deeper experience with the web gui.

I've not worked in this code and I dont really use the feature currently so any insight I can provide wont be the best.

zgredex · 2026-05-11T16:27:01Z

I appreciate the nod towards my ability to review a change like this but I think it best to defer to @itsthisjustin or another whos got deeper experience with the web gui.

I've not worked in this code and I dont really use the feature currently so any insight I can provide wont be the best.

No problem. I’ll fix one more thing and assign more people

itsthisjustin · 2026-05-11T16:58:07Z

Happy to review when needed. I'd run into these issues myself when using the optimizer so I can test

validateFile() opens the image picker on a single-EPUB selection with advanced settings expanded, but had no else branch — re-picking multiple files left the previous file's thumbnails and picker-mode layout visible. Add an else that calls the existing idempotent clearImagePicker() helper so the grid, picker-mode classes, and the Start Conversion button are torn down whenever the selection no longer matches the open-picker condition.

zgredex · 2026-05-11T17:32:19Z

@coderabbitai review

coderabbitai · 2026-05-11T17:33:09Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

zgredex · 2026-05-11T17:42:33Z

@itsthisjustin It should be good now, hope I didn't miss anything. You can test.

From a broader perspective - this needs to become device aware. Currently everything is calculated with 480x800 resolution. If we are to embrace x3 the optimizer needs an update. I don't own x3 though.

itsthisjustin · 2026-05-11T18:22:38Z

ok want me to add support and test?

zgredex · 2026-05-11T18:24:31Z

ok want me to add support and test?

Sure, if you can spare some time.

zgredex requested a review from pablohc May 11, 2026 15:45

coderabbitai Bot reviewed May 11, 2026

View reviewed changes

zgredex changed the title ~~fix: gate EPUB optimiser by extension and report real file sizes~~ fix: gate EPUB optimiser by extension and fix size reporting (single + batch) May 11, 2026

zgredex requested a review from mcrosson May 11, 2026 16:18

zgredex changed the title ~~fix: gate EPUB optimiser by extension and fix size reporting (single + batch)~~ fix: harden EPUB optimiser UI gating, size reporting, and picker teardown May 11, 2026

Uh oh!

Conversation

zgredex commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commits

Test plan

Uh oh!

coderabbitai Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai Bot commented May 11, 2026

Uh oh!

zgredex commented May 11, 2026

Uh oh!

coderabbitai Bot commented May 11, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

zgredex commented May 11, 2026

Uh oh!

coderabbitai Bot commented May 11, 2026

Uh oh!

mcrosson commented May 11, 2026

Uh oh!

zgredex commented May 11, 2026

Uh oh!

itsthisjustin commented May 11, 2026

Uh oh!

zgredex commented May 11, 2026

Uh oh!

coderabbitai Bot commented May 11, 2026

Uh oh!

zgredex commented May 11, 2026

Uh oh!

itsthisjustin commented May 11, 2026

Uh oh!

zgredex commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zgredex commented May 11, 2026 •

edited

Loading

coderabbitai Bot commented May 11, 2026 •

edited

Loading