diff --git a/TODOS_IN_REFACTOR_PLAN.md b/TODOS_IN_REFACTOR_PLAN.md new file mode 100644 index 0000000..1fcfbc3 --- /dev/null +++ b/TODOS_IN_REFACTOR_PLAN.md @@ -0,0 +1,310 @@ +# TODOs and "Do Later" Items in Refactor Plan + +Found **2 technical debt items** that need refinement after initial implementation, plus **5 future enhancements** listed at the end. + +--- + +## Technical Debt (Must Fix Later) + +### 1. Page Content Extraction (CRITICAL) + +**File:** `web/src/reader/formats/reflowable/page-calculator.ts` +**Function:** `getPageContent()` (lines 373-383) +**Issue:** Currently returns entire spine content instead of extracting the actual page slice + +```typescript +// Line 373-383 (CURRENT IMPLEMENTATION) +export function getPageContent(pagination: PaginationData, pageIndex: number): string { + const page = pagination.pageMap.get(pageIndex); + if (!page) return ""; + + const spine = pagination.spineMap.get(page.charStart); + if (!spine) return ""; + + // Extract content between charStart and charEnd + // For now, return full spine content (we'll refine this) + return spine.content; // ⚠️ TECHNICAL DEBT +} +``` + +**What needs to happen:** +```typescript +// TODO: Implement actual HTML slicing +export function getPageContent(pagination: PaginationData, pageIndex: number): string { + const page = pagination.pageMap.get(pageIndex); + if (!page) return ""; + + const spine = pagination.spineMap.get(page.charStart); + if (!spine) return ""; + + // Extract HTML content between page.charStart and page.charEnd + // Need to handle: + // 1. Finding the HTML tag boundaries (don't cut in middle of ) + // 2. Balancing HTML tags + // 3. Preserving inline styles/attributes + // 4. Handling nested elements properly + + // Options: + // a) Use DOMParser to parse, extract slice, re-serialize + // b) Use HTML tag balancing algorithm + // c) Pre-split spine into page-sized chunks during calculation + + return extractHTMLSlice(spine.content, page.charStart, page.charEnd); +} +``` + +**Why this is deferred:** +- HTML slicing is complex and error-prone +- Requires careful tag balancing to avoid broken HTML +- Initial implementation can work with full spine content (pages just show more content) +- Better to test pagination logic first before adding HTML slicing complexity + +**Impact if not fixed:** +- Each "page" will actually show the entire spine item (could be multiple pages worth) +- Page navigation will jump by spine items, not by actual pages +- Progress tracking will be inaccurate +- User won't get true "Kindle-like" discrete pages + +--- + +### 2. Page Content Rendering (CRITICAL) + +**File:** `web/src/reader/formats/reflowable/content-renderer.ts` +**Function:** `renderPage()` (lines 637-676) +**Issue:** Currently renders full spine content instead of page slice + +```typescript +// Line 653-676 (CURRENT IMPLEMENTATION) +export function renderPage( + container: HTMLElement, + content: string, + pageData: PageBoundary | null +): void { + container.innerHTML = ""; + + const wrapper = document.createElement("div"); + wrapper.className = "reflowable-page"; + wrapper.style.height = "calc(100vh - 120px)"; + wrapper.style.overflow = "hidden"; + wrapper.style.position = "relative"; + + // If we have page boundary data, extract that slice + // For now, render full content (we'll refine this) + const contentDiv = document.createElement("div"); + contentDiv.className = "page-content"; + contentDiv.innerHTML = content; // ⚠️ TECHNICAL DEBT - should slice content + contentDiv.style.height = "100%"; + contentDiv.style.overflow = "hidden"; + + wrapper.appendChild(contentDiv); + container.appendChild(wrapper); +} +``` + +**What needs to happen:** +```typescript +export function renderPage( + container: HTMLElement, + content: string, + pageData: PageBoundary | null +): void { + container.innerHTML = ""; + + const wrapper = document.createElement("div"); + wrapper.className = "reflowable-page"; + wrapper.style.height = "calc(100vh - 120px)"; + wrapper.style.overflow = "hidden"; + wrapper.style.position = "relative"; + + const contentDiv = document.createElement("div"); + contentDiv.className = "page-content"; + + // TODO: Extract page slice using pageData.charStart and pageData.charEnd + if (pageData) { + const pageSlice = extractHTMLRange(content, pageData.charStart, pageData.charEnd); + contentDiv.innerHTML = pageSlice; + } else { + contentDiv.innerHTML = content; + } + + contentDiv.style.height = "100%"; + contentDiv.style.overflow = "hidden"; + + wrapper.appendChild(contentDiv); + container.appendChild(wrapper); +} +``` + +**Why this is deferred:** +- Depends on fixing issue #1 (page content extraction) +- Can't render page slices until we can extract them +- Initial implementation will work but show entire spines + +--- + +### 3. Simplified CFI Generation (MEDIUM PRIORITY) + +**File:** `web/src/reader/formats/reflowable/page-calculator.ts` +**Function:** `generateCFI()` (lines 241-248) + +```typescript +// Line 241-248 (CURRENT IMPLEMENTATION) +// Generate CFI for a character position (simplified) +function generateCFI(spineIndex: number, charOffset: number, totalChars: number): string { + // Simplified CFI: epubcfi(/6/4[chap1]!/4/2/1:0) + // For now, use a simple format we can store and restore + return `epubcfi(/6/${spineIndex}!/4/2/1:${charOffset})`; +} +``` + +**What needs to happen:** +- Implement proper EPUB CFI syntax +- Support ID references in spine items +- Support child element paths (not just character offsets) +- Follow [EPUB CFI specification](https://www.w3.org/TR/epub-cfi/) + +**Why simplified version works for now:** +- Custom CFI format can still store/restore positions +- Database stores as string anyway +- Can migrate to proper CFI later without breaking existing data +- Most users won't notice the difference + +**Impact if not fixed:** +- Progress sync with other EPUB readers won't work +- Can't jump to positions from other apps +- Not standards-compliant + +--- + +## Future Enhancements (Optional) + +These are listed in the "Next Steps After Implementation" section: + +### 1. Refine getPageContent ✅ (Already listed above as #1) + +### 2. Add Chapter Boundary Detection + +**Description:** Ensure new chapters always start on a new page (like a physical book) + +**Implementation:** +```typescript +function adjustPageBoundariesForChapters( + pagination: PaginationData, + toc: TOCItem[] +): PaginationData { + // For each TOC entry, ensure it starts at page boundary + // May need to shift pages or add blank pages +} +``` + +**Why optional:** +- Nice-to-have feature +- Doesn't break functionality if absent +- Requires TOC parsing integration + +--- + +### 3. Add Reading Time Estimates + +**Description:** Show "5 min read" based on word count and reading speed + +**From Kavita's implementation:** +```typescript +const WORDS_PER_MINUTE_SLOW = 5000; // 83 words per minute +const WORDS_PER_MINUTE_AVG = 15000; // 250 words per minute +const WORDS_PER_MINUTE_FAST = 30000; // 500 words per minute + +function estimateReadingTime(wordCount: number): { + minutes: number; + range: string; +} { + // Return "5-10 min" format +} +``` + +**Why optional:** +- UX enhancement only +- Simple addition, doesn't affect core functionality + +--- + +### 4. Implement Search Within Book + +**Description:** Full-text search across all pages + +**Implementation:** +- Build search index during pagination +- Map search results to page numbers +- Highlight search terms in content + +**Why optional:** +- Complex feature +- Requires additional UI components +- Can be added incrementally + +--- + +### 5. Add Highlight/Annotation Support + +**Description:** Allow users to highlight text and add notes + +**Implementation:** +- Store annotations in database with CFI positions +- Render highlights in content +- CRUD operations for annotations + +**Why optional:** +- Separate feature from pagination +- Requires backend API work +- Can be implemented independently + +--- + +## Summary + +### Critical (Must Fix Before Production) +1. ✅ **Page content extraction** - Core functionality broken without this +2. ✅ **Page rendering** - Depends on #1 +3. ⚠️ **CFI generation** - Works for now, but should be standards-compliant + +### Optional (Can Ship Without) +1. Chapter boundary detection +2. Reading time estimates +3. Search within book +4. Highlights/annotations + +--- + +## Recommended Implementation Order + +**Phase 1 (Current):** Get basic pagination working +- Use full spine content for now +- Test navigation, progress tracking, page calculation + +**Phase 2 (Next Sprint):** Fix page content extraction +- Implement HTML slicing algorithm +- Fix `getPageContent()` and `renderPage()` +- Test with actual multi-page spines + +**Phase 3 (Later):** Standards compliance +- Implement proper EPUB CFI +- Add chapter boundary detection +- Improve CFI parsing/validation + +**Phase 4 (Future):** UX enhancements +- Reading time estimates +- Search +- Highlights/annotations + +--- + +## Workarounds for Phase 1 + +While page content extraction is not implemented: + +1. **Accept showing full spine content** - Users can still navigate between spines +2. **Smaller spines** - Some EPUBs split content into small files anyway +3. **Pre-split during parsing** - Could split spines into smaller chunks during initial parse (temporary workaround) +4. **CSS overflow: hidden** - At least hides overflow visually, even if content is there + +The critical items should be prioritized immediately after basic pagination is tested and working.