css changes and hacker application speed up#465
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR enhances the Blade hacker dashboard with refined bloomknights leaf/bird animations and responsive viewport-to-scene space transforms, improves hacker form mobile accessibility through safe-area-aware padding and pointer-events navigation controls, optimizes asset loading with preload directives and quality configuration, and updates KHiX social profile links from Facebook/Twitter to GitHub/LinkedIn. ChangesBlade Hacker Dashboard UI
Asset Optimization and Image Loading
Social Links and Branding Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 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.
🧹 Nitpick comments (2)
apps/bloomknights/src/app/_components/graphics/BloomAssetPreloads.tsx (1)
5-5: 💤 Low valueOptional: Remove unnecessary
keyprops.The
keyprops on lines 5 and 13 are not needed here since these<link>elements are not rendered inside a map or array. Keys are only required for list reconciliation in React.♻️ Simplify by removing keys
<> <link - key="bloom-background-mobile" rel="preload" href="https://assets.knighthacks.org/bloom-background-mobile.avif" as="image" type="image/avif" media="(max-width: 767px)" /> <link - key="bloom-background-desktop" rel="preload" href="https://assets.knighthacks.org/bloom-background-desktop.avif" as="image" type="image/avif" media="(min-width: 768px)" /> </>Also applies to: 13-13
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/bloomknights/src/app/_components/graphics/BloomAssetPreloads.tsx` at line 5, In BloomAssetPreloads.tsx (component BloomAssetPreloads) remove the unnecessary key props from the static <link> elements (e.g. key="bloom-background-mobile" and the second key at line 13) since these tags are not part of a mapped list and React keys are only required for list reconciliation; simply delete the key attributes from those <link> elements and keep the rest of the attributes intact.apps/bloomknights/src/app/_components/about/about.tsx (1)
126-126: ⚡ Quick winInconsistency: AI summary claims conditional eager loading, but code loads all images eagerly.
The AI summary states that
loading="eager"is applied only to the first image, but the code showsloading="eager"is unconditional on line 126—both gallery images will load eagerly.While the
fetchPriorityis correctly conditional (high for first, auto for second), setting both images to eager loading may impact performance if the second image is below the fold. Typically, only above-the-fold images should be eager-loaded.🚀 Optimize loading strategy
If only the first image is consistently above the fold, consider making
loadingconditional:<Image src={image.src} alt={image.alt} width={6000} height={4000} sizes={ imageIndex === 0 ? "(min-width: 1536px) 768px, (min-width: 1181px) 464px, (min-width: 768px) 520px, 82vw" : "(min-width: 1536px) 544px, (min-width: 1181px) 320px, (min-width: 768px) 360px, 62vw" } quality={72} - loading="eager" + loading={imageIndex === 0 ? "eager" : "lazy"} fetchPriority={imageIndex === 0 ? "high" : "auto"} className="about-gemiknight-image" />This allows the second image to lazy-load if it's not immediately visible, improving initial page performance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/bloomknights/src/app/_components/about/about.tsx` at line 126, The code unconditionally sets loading="eager" for gallery images in the About component (apps/bloomknights/src/app/_components/about/about.tsx) while fetchPriority is already conditional; change the loading prop to be conditional the same way (e.g., eager for the first/above-the-fold image, lazy for others) so only the primary image loads eagerly and below-the-fold images lazy-load; update the JSX where loading="eager" is used alongside fetchPriority to use a conditional expression tied to the image index or position.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/bloomknights/src/app/_components/about/about.tsx`:
- Line 126: The code unconditionally sets loading="eager" for gallery images in
the About component (apps/bloomknights/src/app/_components/about/about.tsx)
while fetchPriority is already conditional; change the loading prop to be
conditional the same way (e.g., eager for the first/above-the-fold image, lazy
for others) so only the primary image loads eagerly and below-the-fold images
lazy-load; update the JSX where loading="eager" is used alongside fetchPriority
to use a conditional expression tied to the image index or position.
In `@apps/bloomknights/src/app/_components/graphics/BloomAssetPreloads.tsx`:
- Line 5: In BloomAssetPreloads.tsx (component BloomAssetPreloads) remove the
unnecessary key props from the static <link> elements (e.g.
key="bloom-background-mobile" and the second key at line 13) since these tags
are not part of a mapped list and React keys are only required for list
reconciliation; simply delete the key attributes from those <link> elements and
keep the rest of the attributes intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: cf5187b0-f972-4235-bf0c-dbf3ca72b48b
📒 Files selected for processing (9)
apps/blade/src/app/_components/dashboard/hacker/hackbackgrounds/bloomknights.tsapps/blade/src/app/_components/dashboard/hacker/hacker-application-form.tsxapps/bloomknights/next.config.jsapps/bloomknights/src/app/_components/about/about.tsxapps/bloomknights/src/app/_components/graphics/BloomAssetPreloads.tsxapps/bloomknights/src/app/layout.tsxapps/khix/src/app/globals.cssapps/khix/src/app/page.tsxapps/khix/src/app/seo.ts
1 participant
Why
Fixes the BloomKnights/KHIX mobile polish issues from #464. The BloomKnights application background needed to feel like one continuous scene instead of having viewport-pinned effects repeat
What
Closes: #464
Test Plan
git diff --checkpnpm formatpnpm lintpnpm typecheckNODE_ENV=production pnpm buildpnpm lint:wsexits 0 with existing workspace warning for missingpackages/transactional/package.jsonChecklist
Summary by CodeRabbit
New Features
Performance
Updates
Style