Skip to content

fix: center paper theme content relative to viewport#91

Merged
rsbh merged 1 commit into
mainfrom
fix/paper-theme-centered-layout
May 20, 2026
Merged

fix: center paper theme content relative to viewport#91
rsbh merged 1 commit into
mainfrom
fix/paper-theme-centered-layout

Conversation

@rsbh
Copy link
Copy Markdown
Member

@rsbh rsbh commented May 20, 2026

Summary

  • Content area centers to full viewport width using negative margin offset for sidebar
  • Reader mode toggle no longer causes content to shift position
  • Removed padding-right compensation and readerMode CSS class from Page

Test plan

  • Open paper theme — content is centered to viewport, sidebar overlaps left edge
  • Toggle reader mode — content stays in place, no horizontal shift
  • Check landing page (hideSidebar) — content centered without offset

🤖 Generated with Claude Code

Content now centers to full viewport width using negative margin to
offset the sidebar. Reader mode toggle no longer causes content shift.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
chronicle Ready Ready Preview, Comment May 20, 2026 4:18am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 92151d04-1c25-44c8-bd67-8520cc5308a8

📥 Commits

Reviewing files that changed from the base of the PR and between 0679bdf and 9c278f6.

📒 Files selected for processing (4)
  • packages/chronicle/src/themes/paper/Layout.module.css
  • packages/chronicle/src/themes/paper/Layout.tsx
  • packages/chronicle/src/themes/paper/Page.module.css
  • packages/chronicle/src/themes/paper/Page.tsx

📝 Walkthrough

Summary by CodeRabbit

  • Style
    • Refined content layout positioning when the sidebar is visible and hidden.
    • Adjusted main content area width and spacing calculations.
    • Removed reader mode-specific styling from the main content container.

Walkthrough

The paper theme's layout system is updated to handle sidebar visibility and reader mode. Content now applies a negative left margin offset that is conditionally removed when the sidebar is hidden. The main container max-width is simplified to a fixed value and reader-mode-specific styling is removed from the main element.

Changes

Paper Theme Layout and Sizing Adjustments

Layer / File(s) Summary
Sidebar content offset mechanism
packages/chronicle/src/themes/paper/Layout.module.css, packages/chronicle/src/themes/paper/Layout.tsx
The .content class now uses a negative left margin offset by the sidebar width. A .contentFull variant removes this offset. Layout.tsx conditionally applies .contentFull when showSidebar is false.
Main container sizing
packages/chronicle/src/themes/paper/Page.module.css, packages/chronicle/src/themes/paper/Page.tsx
The .main container max-width is simplified from a calc expression to a fixed 1024px value and right padding is removed. Reader-mode-specific styling is no longer conditionally applied to the main element.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • raystack/chronicle#47: Passes hideSidebar for the landing route; this PR changes Layout.tsx/Layout.module.css so the content wrapper switches to contentFull styling when the sidebar is hidden.
  • raystack/chronicle#44: Directly related reader-mode and paper theme redesign work affecting the same Layout content offset and Page main-class styling areas.
  • raystack/chronicle#28: Also adjusts theme-specific page layout widths with max-width: 1024px and centered auto margins on the main content container.

Suggested reviewers

  • rohanchkrabrty
  • rohilsurana
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: centering paper theme content relative to viewport, which is the core objective across all modified files.
Description check ✅ Passed The description is directly related to the changeset, explaining the content centering approach, reader mode behavior, and removed CSS classes, with a clear 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.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/paper-theme-centered-layout

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

Copy link
Copy Markdown
Member

@rohilsurana rohilsurana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issues

  1. Negative margin causes content to overlap the sidebarmargin-left: calc(-1 * var(--paper-sidebar-width)) shifts the content area left by the sidebar width, which centers it relative to the viewport. But this means the content's left edge now sits under the sidebar. The sidebar has position: sticky but no z-index, so the content may visually bleed through or overlap on certain scroll positions. Add z-index: 1 (or similar) to .sidebar to ensure it always paints above the content area.

  2. Responsive breakpoints — The sidebar is likely hidden on small screens (the CSS has no @media queries shown in the diff, but the showSidebar logic handles it). When the sidebar is hidden, contentFull class removes the negative margin — good. But if there's a breakpoint where the sidebar is conditionally hidden via CSS (not just React state), the negative margin would still apply even when the sidebar isn't visible, pushing content off-screen to the left. Verify there are no CSS-only responsive sidebar hides.

  3. max-width: 1024px without padding — The old code had max-width: calc(1024px + var(--rs-space-17)) with padding-right: var(--rs-space-17), giving an effective content width of 1024px with right padding for breathing room. The new code sets max-width: 1024px with no horizontal padding at all. On viewports close to 1024px + sidebar width, the content will touch the edges. Consider adding symmetric horizontal padding (padding: 0 var(--rs-space-7)) to the .main container, or the content may feel cramped.

Looks good overall — the approach is right, just verify the z-index stacking and edge-case viewport widths.

@rsbh rsbh merged commit 9acd409 into main May 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants