Repository navigation
feat(frontend): Lucky brand identity on landing page - #693
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
LucasSantana-Dev
enabled auto-merge (squash)
April 17, 2026 20:31
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR updates the frontend design system with new fonts (Sora, Manrope) and a neon pink/orange color palette, then refactors the Landing page into composable sections with Framer Motion animations, redesigning the hero with logo integration and updating feature and FAQ presentations. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
vi.mock('framer-motion') auto-mocks motion.* to undefined, breaking
render of all 19 Landing tests. Replace with factory that proxies
motion.X to plain HTML elements + stubs AnimatePresence + useReducedMotion.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/frontend/src/pages/Landing.test.tsx (2)
61-70: Make this test assert reduced-motion behavior, not just renderability.The test name says reduced motion is respected, but it only verifies that the page renders. A regression where entrance/scroll animations still run with
prefersReducedMotion: truewould pass here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/Landing.test.tsx` around lines 61 - 70, The test currently only checks renderability; update it to assert that animations/transitions are disabled when setupMocks({ prefersReducedMotion: true }) is used: after render(<Landing />) and locating the container (variable container), call window.getComputedStyle on the element(s) that normally animate (e.g., the container or a specific animated element you can target via class or data-testid) and assert that computedStyle.animationDuration === '0s' and computedStyle.transitionDuration === '0s' (or computedStyle.animationName === 'none'), ensuring the Landing component respects reduced-motion; keep using setupMocks and the same render flow but add these assertions.
135-169: Avoid locking in FAQ answers as always exposed.For an accordion, collapsed answers should not be visible/readable before expansion. These assertions currently require every answer to be present before any click, and the motion passthrough mock won’t apply collapsed
height/opacitystyles. Prefer asserting FAQ buttons render, then verifyaria-expandedand answer visibility after interaction.Example test shape
- faqs.forEach(({ q, a }) => { - expect(screen.getByText(q)).toBeInTheDocument() - expect(screen.getByText(a)).toBeInTheDocument() - }) + faqs.forEach(({ q }) => { + expect(screen.getByRole('button', { name: q })).toBeInTheDocument() + })- const firstQuestion = screen.getByText('Is Lucky free?') - const firstButton = firstQuestion.closest('button') + const firstButton = screen.getByRole('button', { name: 'Is Lucky free?' }) expect(firstButton).toBeInTheDocument() + expect(firstButton).toHaveAttribute('aria-expanded', 'false') - await user.click(firstButton!) + await user.click(firstButton) - const answer = screen.getByText('Yes, Lucky is completely free with no premium tier.') - expect(answer).toBeInTheDocument() + expect(firstButton).toHaveAttribute('aria-expanded', 'true') + expect(screen.getByText('Yes, Lucky is completely free with no premium tier.')).toBeVisible()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/Landing.test.tsx` around lines 135 - 169, The current tests lock in FAQ answers being visible; update both tests to assert questions render as buttons and that answers are hidden until expansion: in the "renders FAQ section..." test (render(<Landing />) and the faqs array) stop asserting screen.getByText(a) for each answer and instead assert each question button exists (e.g., use screen.getByText(q).closest('button') or getByRole) and that its aria-expanded is 'false' initially; in the "FAQ items are expandable via button clicks" test (userEvent.setup(), firstButton) after clicking assert the button's aria-expanded becomes 'true' and then assert the answer text is visible (screen.getByText(a)); keep references to Landing, userEvent.setup(), firstButton, and aria-expanded to locate the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/frontend/src/index.css`:
- Around line 69-73: The runtime aliases (--lucky-brand, --lucky-brand-strong,
--lucky-accent, --lucky-accent-soft) still point to legacy blurple; update their
assignments to reference the new neon semantic tokens (map each --lucky-* alias
to the corresponding --color-lucky-* token) so existing components using
--lucky-* pick up the neon redesign; apply the same mapping fix where these
aliases are defined again (the other occurrence noted around lines 371-381) to
ensure consistency across the stylesheet.
- Line 1: Stylelint is failing because the `@import` uses url(...) notation;
update the import in packages/frontend/src/index.css to use string import
notation instead of url(...) (replace the `@import` line that references the
Google Fonts URL so it uses a plain quoted string); ensure the same font
families and query params are preserved inside the quoted string so the browser
still loads the fonts.
In `@packages/frontend/src/pages/Landing.tsx`:
- Around line 106-111: The hero logo img element (src='/lucky-logo.png',
className='h-32 w-32 mx-auto drop-shadow-2xl filter saturate-150') is being
lazy-loaded; remove the loading='lazy' attribute (or set loading='eager') so the
above-the-fold LCP image loads immediately.
- Around line 132-134: The Button onClick in Landing.tsx currently calls
window.open(BOT_INVITE_URL, '_blank') which leaves window.opener exposed; update
the call to include a feature string (e.g., 'noopener,noreferrer') as the third
argument to window.open to prevent the opened Discord OAuth page from gaining
access to window.opener, and adjust any tests that assert the window.open call
(or its arguments) to expect the feature string as well; locate the handler on
the Button element in Landing.tsx that references BOT_INVITE_URL and update the
test assertion(s) accordingly.
- Around line 249-255: The Users strip incorrectly always appends '+' even when
userCount is 0 or loading; update the value expression in Landing.tsx (the Users
block using statsLoading and userCount) to match the Servers logic by returning
'---' when statsLoading is true and only appending '+' when userCount > 0 (e.g.,
`${userCount.toLocaleString()}${userCount > 0 ? '+' : ''}`), ensuring
consistency with the Servers/guildCount formatting.
- Around line 350-373: The FAQ accordion lacks ARIA state and doesn’t hide
collapsed content from assistive tech; update the button and answer container to
expose state and hide collapsed content: add aria-expanded={openIdx === idx} and
aria-controls={`faq-${idx}`} to the button (function setOpenIdx / variable
openIdx, idx), give the answer wrapper a stable id={`faq-${idx}`} and a11y
role/label (e.g., role="region" and aria-labelledby pointing to the button), and
when closed set aria-hidden="true" (and remove from tab sequence with
tabIndex={-1}) on the answer element and aria-hidden="false" (tabIndex={0}) when
open so screen readers won’t read collapsed answers.
- Around line 56-78: Wrap the page root in Framer Motion's MotionConfig with
reducedMotion="user" and remove scattered manual prefersReducedMotion checks
(e.g., the logoAnimation creation that uses prefersReducedMotion and any feature
hover/entry checks) so all animations (HeroSection, FeatureSection,
StatsSection, FAQSection, FooterSection) automatically respect OS
reduced-motion: replace manual conditionals by relying on MotionConfig
reducedMotion="user" at the top-level component and update/remove per-component
guards (like the logoAnimation logic and hover/entrance guards) so children no
longer need to inspect prefersReducedMotion themselves.
---
Nitpick comments:
In `@packages/frontend/src/pages/Landing.test.tsx`:
- Around line 61-70: The test currently only checks renderability; update it to
assert that animations/transitions are disabled when setupMocks({
prefersReducedMotion: true }) is used: after render(<Landing />) and locating
the container (variable container), call window.getComputedStyle on the
element(s) that normally animate (e.g., the container or a specific animated
element you can target via class or data-testid) and assert that
computedStyle.animationDuration === '0s' and computedStyle.transitionDuration
=== '0s' (or computedStyle.animationName === 'none'), ensuring the Landing
component respects reduced-motion; keep using setupMocks and the same render
flow but add these assertions.
- Around line 135-169: The current tests lock in FAQ answers being visible;
update both tests to assert questions render as buttons and that answers are
hidden until expansion: in the "renders FAQ section..." test (render(<Landing
/>) and the faqs array) stop asserting screen.getByText(a) for each answer and
instead assert each question button exists (e.g., use
screen.getByText(q).closest('button') or getByRole) and that its aria-expanded
is 'false' initially; in the "FAQ items are expandable via button clicks" test
(userEvent.setup(), firstButton) after clicking assert the button's
aria-expanded becomes 'true' and then assert the answer text is visible
(screen.getByText(a)); keep references to Landing, userEvent.setup(),
firstButton, and aria-expanded to locate the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79cfb701-2f7e-437f-ae2b-93ec52b1a8d0
📒 Files selected for processing (3)
packages/frontend/src/index.csspackages/frontend/src/pages/Landing.test.tsxpackages/frontend/src/pages/Landing.tsx
📜 Review details
⏰ 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). (2)
- GitHub Check: Quality Gates
- GitHub Check: SonarCloud Scan
🧰 Additional context used
🪛 Stylelint (17.7.0)
packages/frontend/src/index.css
[error] 1-1: Expected "url('https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700&family=JetBrains+Mono:wght@400;500;600&family=Sora:wght@400;500;600;700&family=Manrope:wght@400;500;600;700&display=swap')" to be "'https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700&family=JetBrains+Mono:wght@400;500;600&family=Sora:wght@400;500;600;700&family=Manrope:wght@400;500;600;700&display=swap'" (import-notation)
(import-notation)
|
LucasSantana-Dev
added a commit
that referenced
this pull request
May 13, 2026
* feat(frontend): redesign landing page with Lucky neon brand identity
* fix(frontend): drop unused prefersReducedMotion + containerAnimation vars
* test(frontend): mock framer-motion with passthrough motion proxy
vi.mock('framer-motion') auto-mocks motion.* to undefined, breaking
render of all 19 Landing tests. Replace with factory that proxies
motion.X to plain HTML elements + stubs AnimatePresence + useReducedMotion.
* test(frontend): set window.open spy before render to capture click
* test(frontend): use fireEvent.click for CTA buttons (userEvent flaky w/ motion mock)
This was referenced May 15, 2026
This branch was successfully deployed
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
Complete redesign of the landing page to showcase Lucky's neon arcade brand identity. No more generic AI-style design.
Visual Changes
Code Changes
Preserved
Before/After
Test locally:
pnpm --filter frontend testshould pass all tests. Build may need full dependency install.Summary by CodeRabbit
New Features
Style