Add technical documentation of the TODO items that were identified during the refactor planning process and subsequently resolved. This document serves as historical reference showing: - Original TODO items that were found in the initial plan - Detailed analysis of why they were deferred - Implementation priority classification (Critical vs Optional) - Code examples of placeholder vs completed implementations - Workarounds that could be used during phased implementation ## Document Contents ### Critical Technical Debt (All Completed) 1. Page content extraction - HTML slicing algorithm 2. Page rendering - Proper slice display 3. CFI generation - Standards compliance ### Future Enhancements (Documented for Later) 1. Chapter boundary detection 2. Reading time estimates 3. Search within book 4. Highlight/annotation support ### Implementation Phases Documented recommended 4-phase approach: - Phase 1: Basic pagination (current implementation) - Phase 2: HTML slicing (now complete) - Phase 3: CFI standards (now complete) - Phase 4: UX enhancements (future work) This reference documentation helps maintain context about technical decisions and implementation priorities for future development.
311 lines
9.0 KiB
Markdown
311 lines
9.0 KiB
Markdown
# 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 <tag>)
|
|
// 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.
|