-
Notifications
You must be signed in to change notification settings - Fork 1
HomePage: "Launchpad" à la place de la liste #98
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
Conversation
WalkthroughAdds a new HomepageLaunchpad component and Storybook story, replaces prior navigation controls in RallyistsHomepage with the launchpad, reorders a Storybook decorator, updates Intro icon usage and translations (EN/FR), and adjusts French wording. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant RH as RallyistsHomepage
participant HL as HomepageLaunchpad
participant QR as QRDrawerContext
participant Router as Router
User->>RH: Open Rallyists homepage
RH->>HL: Render HomepageLaunchpad (when showIntro true)
Note over HL: Two grids — link tiles + QR tile
alt Click QR tile
User->>HL: Click "QR scanner"
HL->>QR: openDrawer()
QR-->>User: QR drawer opens
else Click link tile
User->>HL: Click link tile
HL->>Router: navigate(route)
Router-->>User: Page navigated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
Deploying sedeto-vtube-stamp-rally with
|
| Latest commit: |
faf9fda
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://61b14974.sedeto-vtube-stamp-rally.pages.dev |
| Branch Preview URL: | https://new-homepage-buttons.sedeto-vtube-stamp-rally.pages.dev |
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/src/components/HomepageLaunchpad.stories.tsx(1 hunks)web/src/components/HomepageLaunchpad.tsx(1 hunks)web/src/components/routes/rallyists/RallyistsHomepage.stories.tsx(1 hunks)web/src/components/routes/rallyists/RallyistsHomepage.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
web/src/components/HomepageLaunchpad.stories.tsx (1)
web/src/lib/decorators.tsx (2)
QRDrawerContextProviderDecorator(205-218)RouterDecorator(46-63)
web/src/components/HomepageLaunchpad.tsx (1)
web/src/contexts/useQRDrawerContext.ts (1)
useQRDrawerContext(5-5)
web/src/components/routes/rallyists/RallyistsHomepage.stories.tsx (1)
web/src/lib/decorators.tsx (1)
RouterDecorator(46-63)
🪛 Biome (2.1.2)
web/src/components/HomepageLaunchpad.tsx
[error] 3-3: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (6)
web/src/components/HomepageLaunchpad.tsx (3)
40-58: Sugoi! Nice discriminated union pattern.The type definitions using discriminated unions (
GridElementLinkvsGridElementButton) ensure type safety by makinglinkandonClickmutually exclusive. This is a clean pattern that prevents invalid states at compile time. Sasuga TypeScript~!
63-115: Verify that removing i18n support is intentional.Ara ara~ I notice all the text strings are now hardcoded in French ("Artistes", "Carte", "Récompenses", "Scanner", "Règles", "Paramètres"). Since the AI summary mentions translations were removed from
RallyistsHomepage.tsx, this suggests the app is now French-only.If multi-language support was previously available and is still needed, these strings should be externalised using a translation system. If French-only is the intended direction, then this is perfectly fine!
Could you confirm whether:
- The app is intentionally French-only going forward?
- Or should these strings be prepared for future i18n support?
If i18n is still desired, I can help generate a translation-ready version of this component.
8-38: LGTM! Clean conditional rendering.The
GridItemcomponent elegantly handles both link and button cases with proper type discrimination. The styling and structure are well thought out, and the hover effects (hover:shadow-md) add nice polish to the UI.web/src/components/HomepageLaunchpad.stories.tsx (1)
1-19: Storybook setup looks comfy~!The story configuration is clean and follows Storybook best practices. The decorator order (QRDrawerContextProviderDecorator → RouterDecorator) ensures proper context nesting, and the Default story with an empty object is the standard pattern for components with no required props.
web/src/components/routes/rallyists/RallyistsHomepage.stories.tsx (1)
15-19: Peko! Decorator reordering improves consistency.Moving
RouterDecoratorto the end of the decorators array places it as the outermost wrapper, which makes sense for routing context. This also matches the decorator order inHomepageLaunchpad.stories.tsx, creating consistency across your story files.The new execution order (TanStackQuery → QRDrawerContext → Router) is logically sound!
web/src/components/routes/rallyists/RallyistsHomepage.tsx (1)
5-5: Kawaii~! Clean integration of the new launchpad.The replacement of the old button-based navigation with
HomepageLaunchpadis straightforward and maintains the existingshowIntrologic. The component is properly imported and rendered within a container with appropriate spacing.This refactor simplifies the homepage by delegating navigation to the dedicated launchpad component, which is a nice separation of concerns!
Also applies to: 25-27
7242c3a to
3fcb4e0
Compare
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: 0
♻️ Duplicate comments (1)
web/src/components/HomepageLaunchpad.tsx (1)
1-14: Yatta~! Import organisation is solid.I see the past review concern about
Mapshadowing the global has already been addressed—you're usingMapIconthroughout. The imports are clean and well-organised. Good stuff!
🧹 Nitpick comments (1)
web/src/components/HomepageLaunchpad.tsx (1)
16-46: GridItem component looks great!The conditional rendering logic for Link vs button is clean and type-safe. The component handles both navigation and action items elegantly.
If you want to polish the accessibility a bit more (totally optional!), consider these small tweaks:
- Add explicit
type="button"to the button element (line 42)- Add
aria-hidden="true"to the icon elements since they're decorative (text labels are present)Apply this diff for the button type:
if (link) return <Link to={link}>{content}</Link>; return ( - <button onClick={onClick} className="w-full cursor-pointer"> + <button type="button" onClick={onClick} className="w-full cursor-pointer"> {content} </button>For the icons, you can add
aria-hidden="true"when they're rendered (lines 75, 83, 91, 99, 110, 118), but since they're always paired with text labels, this is really just a nice-to-have polish~
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/components/HomepageLaunchpad.stories.tsx(1 hunks)web/src/components/HomepageLaunchpad.tsx(1 hunks)web/src/components/routes/rallyists/RallyistsHomepage.stories.tsx(1 hunks)web/src/components/routes/rallyists/RallyistsHomepage.tsx(2 hunks)web/src/i18n/fr.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/components/routes/rallyists/RallyistsHomepage.stories.tsx
- web/src/components/routes/rallyists/RallyistsHomepage.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
web/src/components/HomepageLaunchpad.tsx (1)
web/src/contexts/useQRDrawerContext.ts (1)
useQRDrawerContext(5-5)
web/src/components/HomepageLaunchpad.stories.tsx (1)
web/src/lib/decorators.tsx (2)
QRDrawerContextProviderDecorator(205-218)RouterDecorator(46-63)
🔇 Additional comments (4)
web/src/i18n/fr.json (1)
404-404: Sugoi~! Translation update looks clean.The change from "Paramètres" to "Réglages" is a nice refinement for the settings label. Both are valid, but "Réglages" flows better in UI contexts. No concerns here!
web/src/components/HomepageLaunchpad.stories.tsx (1)
1-19: Kawaii~! Your Storybook setup is spot-on.The story structure follows best practices for Storybook 8.x. The decorator order makes perfect sense—QR drawer context first, then routing infrastructure. The empty Default story is exactly what you need for a component with no required props. Sasuga!
web/src/components/HomepageLaunchpad.tsx (2)
48-65: Type definitions are chef's kiss!The discriminated union approach with
GridElementLinkandGridElementButtonis exactly how you handle this kind of conditional logic in TypeScript. The types prevent impossible states (like having bothlinkandonClick), and the base type keeps things DRY. Sasuga developer-san!
67-139: The launchpad implementation is poggers!The main component logic is well-structured:
- Clean separation of the two grids (main navigation 2x2 + secondary options 1x2)
- Proper context usage for the QR drawer
- Translation keys are correctly referenced
- The styling with rounded corners creates a nice cohesive tile aesthetic
- Key generation using
element.titleis appropriateThe overall architecture is solid and maintainable. This launchpad design is going to look really slick in the UI!
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/components/Intro.tsx(2 hunks)web/src/i18n/en.json(1 hunks)web/src/i18n/fr.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/i18n/fr.json
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/components/Intro.tsx (1)
web/src/lib/consts.ts (1)
standardRewardMinStampsRequirement(77-77)
🔇 Additional comments (4)
web/src/i18n/en.json (1)
332-332: Solid Trans-ready translation key! Sasuga!The template structure with the
<1>placeholder for strong emphasis and{{minimumStampsCount}}variable is spot-on for the Trans component wiring in Intro.tsx. Chat will understand exactly where to go for their gacha—I mean, reward!web/src/components/Intro.tsx (3)
1-2: Clean import optimization! Very seiso!Narrowing the lucide-react imports to just the icons you're actually using and adding Trans for i18n interpolation keeps the bundle nice and tidy. Good housekeeping in the codebase!
4-4: Import path simplification looks good!Dropping the
.tsextension is perfectly valid for TypeScript module resolution. The module system handles this automatically, so this change is poggers.
21-30: Trans component wiring is chef's kiss! 草The Trans component is properly configured with:
- Correct
tandi18nKeyprops- Strong wrapper for inline emphasis via
components={{ 1: <strong /> }}minimumStampsCountpassed throughvaluespropThis matches perfectly with the translation structure in en.json (once that grammar issue is fixed). The stamp count will dynamically render with proper emphasis, making it crystal clear for rally participants!
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
♻️ Duplicate comments (1)
web/src/i18n/en.json (1)
331-331: Sentence still incomplete, senpai noticed~The past review comment is still valid - the sentence ends with "participating artists" but doesn't specify what they're participating in. It's like ending a stream mid-sentence, chat would be confused!
Apply this diff to complete the sentence:
- "1": "Get a stamp by making a purchase (minimum 2€) from one of the 16 participating artists", + "1": "Get a stamp by making a purchase (minimum 2€) from one of the 16 artists participating in the rally",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/routes/rallyists/Rules.test.tsx(1 hunks)web/src/i18n/en.json(1 hunks)
🔇 Additional comments (1)
web/src/components/routes/rallyists/Rules.test.tsx (1)
19-19: LGTM! Test updated to match the new rally description, poi~The assertion correctly reflects the condensed rally description that went from 6 points to 2 points. This change keeps the test in sync with the translation updates - totally seiso!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Refactor
Documentation
UI / Content
Localization
Tests