Removed large assets, replaced media with CDN links#350
Conversation
|
""" WalkthroughAll image and video asset URLs in the LandingPage, Developers, Footer, Header, GlobalStyles, and WaitList components were updated from local relative paths to absolute URLs referencing a CDN. A new splitChunks optimization configuration was added in config-overrides.js to manage vendor chunk size. The GitHub Actions workflow for downloading the latest token list was removed entirely. Changes
Suggested reviewers
Poem
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
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/pages/Landing.jsx (6)
24-27: Add lazy loading for hero imageDeferring off-screen images can improve initial page load performance.
Apply this diff:
@@ 24-27 - <img - src="https://cdn.pillarx.app/home-px.png" - alt="PillarX logo" - /> + <img + src="https://cdn.pillarx.app/home-px.png" + alt="PillarX logo" + loading="lazy" + decoding="async" + />
65-68: Provide fallback content for unsupported browsers in videoIncluding fallback markup inside each
<video>improves accessibility for formats not supported by the user’s browser.Example diff:
@@ 58-70 - <video - width="100%" - preload="none" - autoPlay - loop - muted - playsInline - poster="https://cdn.pillarx.app/home-feature-1.png" - > - <source - src="https://cdn.pillarx.app/home-feature-1.mp4" - type="video/mp4" - /> - </video> + <video + width="100%" + preload="none" + autoPlay + loop + muted + playsInline + poster="https://cdn.pillarx.app/home-feature-1.png" + > + <source + src="https://cdn.pillarx.app/home-feature-1.mp4" + type="video/mp4" + /> + {/* Fallback: browser does not support embedded videos */} + Your browser does not support embedded videos. + </video>
93-97: Provide fallback content in second feature videoMirror the accessibility enhancement by adding a fallback message in this
<video>block.@@ 86-98 - <video - width="100%" - preload="none" - autoPlay - loop - muted - playsInline - poster="https://cdn.pillarx.app/home-feature-2.png" - > - <source - src="https://cdn.pillarx.app/home-feature-2.mp4" - type="video/mp4" - /> - </video> + <video + width="100%" + preload="none" + autoPlay + loop + muted + playsInline + poster="https://cdn.pillarx.app/home-feature-2.png" + > + <source + src="https://cdn.pillarx.app/home-feature-2.mp4" + type="video/mp4" + /> + {/* Fallback: browser does not support embedded videos */} + Your browser does not support embedded videos. + </video>
119-123: Provide fallback content in third feature videoEnsure this
<video>tag also includes fallback text for unsupported environments.@@ 111-125 - <video - width="100%" - preload="none" - autoPlay - loop - muted - playsInline - poster="https://cdn.pillarx.app/home-feature-3.png" - > - <source - src="https://cdn.pillarx.app/home-feature-3.mp4" - type="video/mp4" - /> - </video> + <video + width="100%" + preload="none" + autoPlay + loop + muted + playsInline + poster="https://cdn.pillarx.app/home-feature-3.png" + > + <source + src="https://cdn.pillarx.app/home-feature-3.mp4" + type="video/mp4" + /> + {/* Fallback: browser does not support embedded videos */} + Your browser does not support embedded videos. + </video>
145-147: Add lazy loading for dashboard imageDeferring non-critical images helps reduce initial payload.
@@ 144-147 - <img - src="https://cdn.pillarx.app/home-hero.png" - alt="PillarX Dashboard" - /> + <img + src="https://cdn.pillarx.app/home-hero.png" + alt="PillarX Dashboard" + loading="lazy" + decoding="async" + />
214-217: Refine alt text and defer logo loadingThe current
alt="home-logo"is generic—use a descriptive label, and add lazy loading for performance.@@ 214-217 - <img - src="https://cdn.pillarx.app/home-logo-px.svg" - alt="home-logo" - /> + <img + src="https://cdn.pillarx.app/home-logo-px.svg" + alt="PillarX logo" + loading="lazy" + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
public/landing-images/bottomRightShadow.pngis excluded by!**/*.pngpublic/landing-images/centerLineGradient.pngis excluded by!**/*.pngpublic/landing-images/home-feature-1.gifis excluded by!**/*.gifpublic/landing-images/home-feature-1.mp4is excluded by!**/*.mp4public/landing-images/home-feature-1.pngis excluded by!**/*.pngpublic/landing-images/home-feature-2.gifis excluded by!**/*.gifpublic/landing-images/home-feature-2.mp4is excluded by!**/*.mp4public/landing-images/home-feature-2.pngis excluded by!**/*.pngpublic/landing-images/home-feature-3.gifis excluded by!**/*.gifpublic/landing-images/home-feature-3.mp4is excluded by!**/*.mp4public/landing-images/home-feature-3.pngis excluded by!**/*.pngpublic/landing-images/home-hero.pngis excluded by!**/*.pngpublic/landing-images/home-logo-px.svgis excluded by!**/*.svgpublic/landing-images/topBottomGradient.pngis excluded by!**/*.pngpublic/landing-images/topShadow.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/pages/Landing.jsx(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
Deploying x with
|
| Latest commit: |
6d0b24d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bdcade2e.x-e62.pages.dev |
| Branch Preview URL: | https://improve-cdn-assets.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (improve/cdn-assets)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config-overrides.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (improve/cdn-assets)
github-actions
bot
requested a deployment
to
Preview (improve/cdn-assets)
github-actions
bot
temporarily deployed
to
Preview (improve/cdn-assets)
github-actions
bot
requested a deployment
to
Preview (improve/cdn-assets)
github-actions
bot
temporarily deployed
to
Preview (improve/cdn-assets)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
config-overrides.js (1)
24-29: The TerserPlugin removal may still impact crypto functionality.The commented out TerserPlugin was specifically handling minification for the
@noble/hashes/esm/sha3.jscryptographic library. While the new module filtering approach (lines 31-48) excludes the file entirely, this is a different solution that may not address the original minification concerns.Please confirm that the new module exclusion approach maintains the same cryptographic functionality as the previous TerserPlugin configuration.
🧹 Nitpick comments (1)
config-overrides.js (1)
31-48: Apply optional chaining and clarify the exclusion strategy.The module filtering approach excludes the crypto file entirely from processing, which differs from the previous minification-specific handling.
Apply this diff to use optional chaining as suggested by static analysis:
- if ( - oneOfRule.test && - oneOfRule.test.toString().includes('node_modules') - ) { + if (oneOfRule.test?.toString().includes('node_modules')) { return !oneOfRule.test .toString() .includes('@noble/hashes/esm/sha3.js'); }Additionally, consider adding a comment explaining why this specific crypto file needs to be excluded from module processing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config-overrides.js(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
config-overrides.js
[error] 37-38: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (1)
config-overrides.js (1)
24-48: Clarify the relationship between webpack changes and CDN optimization.The webpack configuration changes (commenting out TerserPlugin and adding module filtering) don't appear directly related to the PR's main objective of replacing media assets with CDN links.
Please clarify:
- Are these webpack changes necessary for the CDN asset optimization?
- Were these changes made to fix build issues encountered during the migration?
- Should these configuration changes be in a separate PR focused on build optimization?
This will help ensure the changes are properly documented and tested in the appropriate context.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit