fix: remove Suspense from hero splash logo#786
Open
franklinjavier wants to merge 4 commits intoTanStack:mainfrom
Open
fix: remove Suspense from hero splash logo#786franklinjavier wants to merge 4 commits intoTanStack:mainfrom
franklinjavier wants to merge 4 commits intoTanStack:mainfrom
Conversation
The LazyBrandContextMenu Suspense boundary caused layout shift on page load as the fallback div swapped to the loaded component. Since both rendered the same images, replace with direct rendering.
👷 Deploy request for tanstack pending review.Visit the deploys page to approve it
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved the React.lazy/React.Suspense lazy-loading path for BrandContextMenu in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/index.tsx`:
- Around line 131-143: The dark-theme NetlifyImage component (the second
NetlifyImage instance rendering splash-dark.png) is currently set to
loading="eager" and fetchPriority="high" even though it is hidden via CSS;
change that component's props to loading="lazy" and fetchPriority="auto" so the
hidden theme variant isn't fetched with high priority. Locate the NetlifyImage
with src="/images/logos/splash-dark.png" and update its loading and
fetchPriority attributes accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Only one splash image is visible per theme but both were fetched eagerly. Keep only the light variant (works in both themes) to avoid wasting bandwidth on a hidden image.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/index.tsx`:
- Around line 124-133: Restore the dark-mode splash by adding the paired dark
image alongside the existing <NetlifyImage> (keep the current light src
"/images/logos/splash-light.png" and add a dark variant
"/images/logos/splash-dark.png"), using the same sizing/props but with
className="dark:hidden" on the light image and className="hidden dark:block" on
the dark image; lazy-load the dark variant (e.g., loading="lazy" / lower
fetchPriority) to avoid CLS while keeping the light image eager, and ensure both
images keep the same layout classes (w-[300px] pt-8 xl:pt-0 xl:w-[400px]
2xl:w-[500px]) so switching themes doesn't shift layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Re-add the dark variant splash image with loading="lazy" so it doesn't compete with the light image for bandwidth. Light image stays eager for LCP; dark image loads on demand when dark mode is active.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/index.tsx`:
- Around line 124-142: Restore the BrandContextMenu wrapper around the hero
splash images so right‑click/context actions are preserved: replace the direct
NetlifyImage renderings inside the hero with <BrandContextMenu> wrapping the
NetlifyImage elements (the two NetlifyImage instances shown) and remove any
Suspense/React.lazy usage for BrandContextMenu so it renders synchronously;
ensure you import and use the BrandContextMenu component where NetlifyImage is
currently rendered and keep the image props (src, width, height, className, alt,
loading, etc.) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Keep the right-click brand actions on the hero splash logo while still eliminating the Suspense fallback→loaded layout shift that caused CLS. The component is now a normal import instead of React.lazy, so it renders on the first pass with no swap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
React.Suspense/LazyBrandContextMenuwrapper around the hero splash logo images<div>vs<BrandContextMenu>), causing layout shift on every page loadTest plan
Summary by CodeRabbit