-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat(frontend): add home hero #16
Conversation
WalkthroughThe changes in this pull request introduce several enhancements to a Next.js application. A new image configuration is added to optimize remote images, while a new button component is created with flexible styling options. Additionally, a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Home
participant Hero
participant GraphQL
User->>Home: Request Home Page
Home->>GraphQL: Fetch heroData
GraphQL-->>Home: Return heroData
Home->>Hero: Render Hero Component with heroData
Hero-->>User: Display Hero Section
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
✅ Deploy Preview for kleros-website-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 7
🧹 Outside diff range and nitpick comments (4)
frontend/src/components/Button.tsx (2)
27-36
: Consider simplifying the className compositionThe current implementation is correct, but the className composition could be more concise.
- className={ - clsx(variant === "primary" ? primaryStyle : secondaryStyle, className) - } + className={clsx( + variant === "primary" ? primaryStyle : secondaryStyle, + className + )}
19-25
: Consider extending button functionality for common use casesAs this appears to be a foundational button component, consider adding these commonly needed features:
type
prop for form submission control (submit
,button
,reset
)disabled
state handling- Loading state for async operations
- Icon support (left/right)
Example interface extension:
interface IButton { children: React.ReactNode; onClick?: () => void; variant?: "primary" | "secondary"; className?: string; type?: "button" | "submit" | "reset"; disabled?: boolean; isLoading?: boolean; leftIcon?: React.ReactNode; rightIcon?: React.ReactNode; }frontend/src/pages/index.tsx (1)
18-18
: Consider more explicit prop passing.While
{...{ heroData }}
works, direct prop passing would be more readable.- <Hero {...{ heroData }} /> + <Hero heroData={heroData} />frontend/src/components/home/Hero.tsx (1)
1-12
: Consider organizing imports and improving interface nameConsider organizing imports into groups (external/internal) and using a more descriptive interface name like
IHeroProps
orHeroProps
to better indicate its purpose as props interface.import React from "react"; - import Image from "next/image"; import Link from "next/link"; +import Button from "@/components/Button"; import LinkArrow from "@/assets/svgs/icons/link-arrow.svg" -import Button from "@/components/Button"; import { HeroQueryType } from "@/queries/home/hero" -interface IHero { +interface HeroProps { heroData: HeroQueryType["homePageHero"]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
frontend/src/assets/svgs/icons/link-arrow.svg
is excluded by!**/*.svg
📒 Files selected for processing (6)
frontend/next.config.mjs
(1 hunks)frontend/src/components/Button.tsx
(1 hunks)frontend/src/components/home/Hero.tsx
(1 hunks)frontend/src/pages/index.tsx
(2 hunks)frontend/src/queries/home/hero.tsx
(1 hunks)frontend/tailwind.config.ts
(1 hunks)
🔇 Additional comments (10)
frontend/next.config.mjs (2)
4-11
: LGTM! Secure image configuration.
The configuration properly restricts remote images to HTTPS protocol and a specific hostname, following Next.js security best practices.
8-8
: Verify if this is the production Strapi hostname.
The hostname "delicate-eggs-595f0e58b8.media.strapiapp.com" appears to be an auto-generated development URL. Please confirm if this is the correct production hostname to avoid deployment issues.
frontend/src/components/Button.tsx (2)
23-23
: LGTM! Well-typed interface extension
The variant prop is properly typed with a union type, making it type-safe and self-documenting.
5-17
:
Consider accessibility improvements for button styles
While the styling structure is well-organized, there are potential accessibility concerns:
- No explicit text color is defined, which could lead to poor contrast ratios
- The secondary button's transparent background might not provide sufficient contrast in all contexts
Consider adding these improvements:
const baseStyle = clsx("px-8 py-2 rounded-full transition duration-75");
const primaryStyle = clsx(
baseStyle,
"bg-primary-blue",
+ "text-white",
"hover:bg-primary-blue/90"
);
const secondaryStyle = clsx(
baseStyle,
"bg-transparent border-2 border-white",
+ "text-white",
"hover:bg-white/10"
);
frontend/tailwind.config.ts (1)
10-17
: Verify the larger font sizes in responsive layouts.
The font size configuration follows good practices with rem units and appropriate line heights. However, the larger sizes (especially 4xl at 6rem/96px) should be tested across different viewport sizes.
Let's verify the usage of these large font sizes:
frontend/src/queries/home/hero.tsx (2)
1-1
: LGTM!
Clean and focused import statement.
33-59
: 🛠️ Refactor suggestion
Consider improving type safety and reusability.
- Extract the button type to reduce duplication
- Consider URL validation for security
Here's a suggested refactor:
+type ButtonType = {
+ text: string;
+ link: {
+ url: string;
+ };
+};
+
export type HeroQueryType = {
homePageHero: {
title: string;
subtitle: string;
- primaryButton: {
- text: string;
- link: {
- url: string;
- };
- };
- secondaryButton: {
- text: string;
- link: {
- url: string;
- };
- };
- arrowLink: {
- text: string;
- link: {
- url: string;
- };
- };
+ primaryButton: ButtonType;
+ secondaryButton: ButtonType;
+ arrowLink: ButtonType;
background: {
url: string;
};
};
};
Let's verify if URL validation is implemented elsewhere:
frontend/src/pages/index.tsx (3)
2-2
: LGTM: Imports are well-organized.
The new imports follow the project's conventions and are logically grouped.
Also applies to: 5-5
12-12
: LGTM: Type-safe interface extension.
Good use of index access type to ensure type safety with the GraphQL response.
15-15
: LGTM: Component signature properly updated.
The signature correctly reflects the interface changes and follows React best practices.
Summary by CodeRabbit
Release Notes
New Features
Hero
component for the homepage, displaying a title, subtitle, and buttons with enhanced styling options.Bug Fixes
primary
andsecondary
variants for better visual consistency.Documentation