-
Notifications
You must be signed in to change notification settings - Fork 0
fix: preserve manual line breaks and match Notion spacing #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Testing UpdateAdded comprehensive test coverage for line break preservation in commit f4e7f66:
Visual Comparison NeededThe CSS spacing changes in To capture screenshots:1. Revert CSS changes temporarily: git stash
# Or checkout the CSS file from main:
# git checkout main -- src/css/custom.css2. Start dev server: bun run dev3. Capture BEFORE screenshots: # Example for a docs page with paragraphs and lists
bun scripts/screenshot-prs.ts --url /docs/overview --name before-spacing4. Re-apply CSS changes: git stash pop
# Or re-checkout this branch's version:
# git checkout fix/preserve-line-breaks-and-spacing -- src/css/custom.css5. Capture AFTER screenshots: # Same page as before
bun scripts/screenshot-prs.ts --url /docs/overview --name after-spacing6. Upload screenshots:
Expected ChangesThe CSS modifications reduce vertical spacing to match Notion's layout:
The visual difference should show more compact, readable content that better matches the source Notion pages. |
🚀 Preview DeploymentYour documentation preview is ready! Preview URL: https://pr-57.comapeo-docs.pages.dev This preview will update automatically when you push new commits to this PR. Built with commit 4c90b1b |
Code Review SummaryWhat This PR DoesThis PR addresses two separate but related issues in the content rendering pipeline: Issue #47 - Manual Line Breaks Preservation:
Issue #48 - Spacing Alignment:
How It Solves the Issues
Potential Issues & ConsiderationsStrengths:
Areas to Watch:
Code Quality:
Recommendation: ✅ Approve - Good implementation with solid test coverage. Just coordinate with PR #51 to avoid CSS conflicts. |
|
@codex pull from main resolving conflicts and review this PR making sure it's ready for merge. |
|
Summary
Testing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
comapeo-docs/scripts/notion-fetch/exportDatabase.ts
Lines 293 to 304 in 1b7eeb4
| function analyzeBlock(block: Record<string, any>): BlockAnalysis { | |
| const blockType = block.type || "unknown"; | |
| const textContent = extractTextFromBlock(block); | |
| const hasContent = textContent.length > 0 && textContent.trim() !== ""; | |
| const hasChildren = | |
| block.has_children || (block.children && block.children.length > 0); | |
| const childrenCount = hasChildren ? block.children?.length || 0 : 0; | |
| return { | |
| type: blockType, | |
| hasContent, | |
| contentLength: textContent.length, |
<br /> in content scoring
Manual line breaks are now converted to "<br />\n" inside extractTextFromBlock, but analyzeBlock immediately uses the mutated string to compute contentLength. Each former newline now contributes six extra characters, so blocks with many line breaks (especially code blocks) will score much higher than before and can exceed thresholds like >50 or >100 characters without having additional textual content. This inflates the contentScore, totalTextLength, and related analytics emitted by exportNotionDatabase, defeating the purpose of those metrics. Consider measuring length before injecting <br /> or strip the HTML tags when calculating scores.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7fe544a to
d63c759
Compare
🧹 Preview Deployment CleanupThe preview deployment for this PR has been cleaned up. Preview URL was: Note: Cloudflare Pages deployments follow automatic retention policies. Old previews are cleaned up automatically. |
Summary
This PR fixes two related content rendering issues to improve consistency between Notion and published documentation:
Changes
Issue #47: Manual Line Breaks
scripts/notion-fetch/exportDatabase.tsto preserve manual line breaks (Shift+Enter) from Notion\n,\u2028,\u2029) to HTML<br />tagsIssue #48: Spacing Adjustments
src/css/custom.cssto reduce paragraph and list spacingmargin-bottomfrom ~1rem to 0.5remmargin-bottomto 0.75rem.theme-doc-markdownto avoid affecting other componentsTesting
runFetchPipeline.test.ts)Manual Testing Needed
Notes
<br />)exportDatabase.tsare unrelated to these changesCloses #47
Closes #48