-
Notifications
You must be signed in to change notification settings - Fork 7
Reduce CLS #332
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
Reduce CLS #332
Conversation
- Set min-height on main.article to prevent layout shift as content loads - Set footer min-height to match reserved space in layout - Minor: update logo width in header
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adjusts layout and rendering across CSS and Handlebars templates. It adds min-height to the footer and converts main.article into a vertical flex container with a viewport-based min-height, plus responsive tweaks for TOC visibility and content overflow. It updates Algolia search preview styles to reserve fixed space, replacing a prior margin-only rule (in both search.css and search-bump.css). Head markup now preloads five WOFF2 fonts with crossorigin. The header logo image gets fixed width/height and an aspect-ratio style. No control flow or templating logic changes are introduced. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/css/footer.css (1)
11-11
: Replace magic number with a shared variable to avoid drift with main.article calc.Use a CSS custom property so footer height stays in sync across files.
Apply this diff:
- min-height: 60px; + min-height: var(--footer-min-height);Add once in a global variables scope (e.g., :root or your variables file):
:root { --footer-min-height: 60px; }src/css/search-bump.css (1)
192-199
: Parametrize Algolia preview sizing; reduce duplication risk with search.css.Same block exists in search.css. Use CSS vars with sensible fallbacks so both bundles stay consistent.
Apply this diff:
-/* Reserve space for Algolia preview panel to prevent CLS */ -.aa-Preview.aa-Column.doc { - min-width: 350px; - max-width: 600px; - min-height: 400px; - margin-top: 0; -} +/* Reserve space for Algolia preview panel to prevent CLS */ +.aa-Preview.aa-Column.doc { + min-width: var(--aa-preview-min-width, 350px); + max-width: var(--aa-preview-max-width, 600px); + min-height: var(--aa-preview-min-height, 400px); + margin-top: 0; +}Please confirm whether both search.css and search-bump.css are loaded together. If yes, consider keeping this rule in a single bundle or introduce the vars above in one place to avoid future drift.
src/partials/header-content.hbs (1)
7-7
: Good CLS fix with explicit dimensions; consider async decoding hint.Width/height + aspect-ratio is solid. Add decoding=async to avoid blocking paint.
Apply this diff:
- <a class="navbar-item" href="{{{relativize this}}}" aria-label="Go to home page"><img src="{{{@root.uiRootPath}}}/img/redpanda-docs-logo-white.svg" width="150" height="22" style="aspect-ratio:150/22;" alt="Redpanda logo"/></a> + <a class="navbar-item" href="{{{relativize this}}}" aria-label="Go to home page"><img src="{{{@root.uiRootPath}}}/img/redpanda-docs-logo-white.svg" width="150" height="22" style="aspect-ratio:150/22;" decoding="async" alt="Redpanda logo"/></a>src/css/main.css (2)
2-2
: Use the same footer variable here to keep layout math single-sourced.Align with footer.css to prevent subtle CLS from mismatched values.
Apply this diff:
- min-height: calc(100vh - var(--body-top) - 60px); + min-height: calc(100vh - var(--body-top) - var(--footer-min-height));
12-15
: Prevent flex child overflow on mobile Safari.When main.article is a flex container, some children can overflow. Add min-height: 0 to the content area.
Add (outside this hunk):
main > .content { min-height: 0; }Check short-content pages at ~768–1024px to ensure no extra vertical scrollbar appears due to the new calc/min-heights.
src/css/search.css (1)
245-252
: DRY up Algolia preview sizing and sync with search-bump.css.Mirror the variable-based rule so both bundles stay aligned.
Apply this diff:
-/* Reserve space for Algolia preview panel to prevent CLS */ -.aa-Preview.aa-Column.doc { - min-width: 350px; - max-width: 600px; - min-height: 400px; - margin-top: 0; -} +/* Reserve space for Algolia preview panel to prevent CLS */ +.aa-Preview.aa-Column.doc { + min-width: var(--aa-preview-min-width, 350px); + max-width: var(--aa-preview-max-width, 600px); + min-height: var(--aa-preview-min-height, 400px); + margin-top: 0; +}On ~900–1100px widths, confirm the grid (45% 1fr) doesn’t force the preview below its min-width, causing horizontal scroll.
src/partials/head.hbs (2)
6-10
: Move preloads earlier in to maximize impact.Place these links before {{> head-styles}} so the browser can fetch fonts in parallel with CSS, not after it.
Apply:
{{> head-info}} -{{> head-styles}} -<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-Regular.woff2" as="font" type="font/woff2" crossorigin> -<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-medium.woff2" as="font" type="font/woff2" crossorigin> -<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-bold.woff2" as="font" type="font/woff2" crossorigin> -<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-semibold.woff2" as="font" type="font/woff2" crossorigin> -<link rel="preload" href="{{{@root.uiRootPath}}}/font/Patron-Medium.woff2" as="font" type="font/woff2" crossorigin> +<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-Regular.woff2" as="font" type="font/woff2" crossorigin> +<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-medium.woff2" as="font" type="font/woff2" crossorigin> +<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-bold.woff2" as="font" type="font/woff2" crossorigin> +<link rel="preload" href="{{{@root.uiRootPath}}}/font/calibre-semibold.woff2" as="font" type="font/woff2" crossorigin> +<link rel="preload" href="{{{@root.uiRootPath}}}/font/Patron-Medium.woff2" as="font" type="font/woff2" crossorigin> +{{> head-styles}}
6-10
: Scope preloads to critical, above-the-fold fonts; add preconnect if cross-origin.If all five aren’t needed at first paint, preload only the body + headings weights to reduce contention; rely on font-display for the rest. If fonts are served from a different origin, add a preconnect with crossorigin to that origin in head.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/css/footer.css
(1 hunks)src/css/main.css
(1 hunks)src/css/search-bump.css
(1 hunks)src/css/search.css
(1 hunks)src/partials/head.hbs
(1 hunks)src/partials/header-content.hbs
(1 hunks)
⏰ 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
🔇 Additional comments (1)
src/partials/head.hbs (1)
6-10
: LGTM: correct preload attributes for fonts.Using rel="preload" with as="font", type="font/woff2", and crossorigin is correct and should help reduce CLS from late font swaps.
I recently checked our CLS score in Google Search Console and while all of our pages on mobile are good, on desktop they are all poor.
Using Chrome dev tools, I narrowed down the culrpit to the
<main>
element.After these changes, the CLS is significantly reduced.
This pull request introduces several improvements to the site's layout and performance, with a focus on search UI stability, layout consistency, and font loading optimization. The most significant changes are grouped below:
Layout and UI Consistency:
min-height
to thefooter.footer
and updatedmain.article
to ensure the main content area always fills the viewport, improving page structure and preventing layout issues when content is short. [1] [2]Search UI Stability:
search.css
andsearch-bump.css
to reserve space and prevent Cumulative Layout Shift (CLS), resulting in a more stable search experience. [1] [2]Performance Optimization:
head.hbs
to improve font loading performance and reduce layout shifts caused by late font rendering.