-
Notifications
You must be signed in to change notification settings - Fork 0
Fix(frontend)/apply feedback round 1 #90
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
base: master
Are you sure you want to change the base?
Changes from all commits
81a550b
8d87ef0
d72bff4
e7cef8d
c107b2e
68bcee2
d3e6ef8
0bf61ec
b7681c0
2d10990
1fb43e1
38de70c
5983784
eea4ab4
be7e2c0
7e4090e
74d98a0
2a1857a
644dc2e
fdafe15
0d3540b
af89a8d
f3b24be
72adb30
a624af6
eb916d6
28ebf5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import React from "react"; | ||
|
||
import Image from "next/image"; | ||
|
||
import { request } from "@/utils/graphQLClient"; | ||
|
||
import { heroQuery, HeroQueryType } from "../queries/hero"; | ||
|
||
const Hero: React.FC = async () => { | ||
const { header, subtitle, background } = ( | ||
await request<HeroQueryType>(heroQuery) | ||
).forLawyersPageHero; | ||
|
||
return ( | ||
<div className="relative px-6 pb-64 pt-44 md:pt-52 lg:px-32 lg:pb-80"> | ||
<div className="space-y-8"> | ||
<h1 | ||
className={ | ||
"pt-1 text-2xl font-medium text-primary-text lg:pt-3 lg:text-4xl" | ||
} | ||
> | ||
{header} | ||
</h1> | ||
<p className="max-w-[685px] text-lg text-primary-text">{subtitle}</p> | ||
</div> | ||
<Image | ||
src={background.url} | ||
alt="Hero Image Background" | ||
fill | ||
unoptimized | ||
className="absolute left-0 top-0 z-[-1] h-full object-cover" | ||
/> | ||
</div> | ||
); | ||
}; | ||
|
||
export default Hero; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import _HighlightedText from "@/components/HighlightedText"; | ||
|
||
import { HighlightedText as IHighlightedText } from "../../queries/kleros-enterprise-section"; | ||
|
||
const HighlightedText: React.FC< | ||
IHighlightedText & { fullTextStyle?: string; highlightedTextStyle?: string } | ||
> = ({ fullText, highlightedText, fullTextStyle, highlightedTextStyle }) => { | ||
return ( | ||
<_HighlightedText | ||
{...{ fullText, highlightedText, fullTextStyle, highlightedTextStyle }} | ||
/> | ||
); | ||
}; | ||
|
||
export default HighlightedText; | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||||||||||||||
import clsx from "clsx"; | ||||||||||||||||||||||||||||||||||||||
import Image from "next/image"; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
import { request } from "@/utils/graphQLClient"; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
import { industriesQuery, IIndustriesQuery } from "./queries"; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
const Industries: React.FC = async () => { | ||||||||||||||||||||||||||||||||||||||
const industriesData = await request<IIndustriesQuery>(industriesQuery); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for the GraphQL request. Your async component correctly fetches data, but there's no error handling. If the request fails, it would cause the entire component to fail rendering. const Industries: React.FC = async () => {
- const industriesData = await request<IIndustriesQuery>(industriesQuery);
+ try {
+ const industriesData = await request<IIndustriesQuery>(industriesQuery);
+
+ return (
+ <div className="flex flex-wrap gap-6 lg:mx-32">
+ {industriesData.enterprise.industries.map(({ title, icon }) => (
+ // Component rendering...
+ ))}
+ </div>
+ );
+ } catch (error) {
+ console.error("Failed to fetch industries data:", error);
+ return <div className="text-center p-6">Unable to load industries information.</div>;
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||
<div className="flex flex-wrap gap-6 lg:mx-32"> | ||||||||||||||||||||||||||||||||||||||
{industriesData.enterprise.industries.map(({ title, icon }) => ( | ||||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||||
className={clsx( | ||||||||||||||||||||||||||||||||||||||
"flex flex-1 flex-col items-center rounded-2xl border", | ||||||||||||||||||||||||||||||||||||||
"border-stroke p-6", | ||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||
key={title} | ||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||
<Image | ||||||||||||||||||||||||||||||||||||||
className="mx-10" | ||||||||||||||||||||||||||||||||||||||
src={icon.url} | ||||||||||||||||||||||||||||||||||||||
alt={title + " card image"} | ||||||||||||||||||||||||||||||||||||||
width="72" | ||||||||||||||||||||||||||||||||||||||
height="72" | ||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||
<p className="mx-auto w-max text-lg text-primary-text lg:text-xl"> | ||||||||||||||||||||||||||||||||||||||
{title} | ||||||||||||||||||||||||||||||||||||||
</p> | ||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
export default Industries; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { gql } from "graphql-request"; | ||
|
||
export const industriesQuery = gql` | ||
{ | ||
enterprise { | ||
industries { | ||
title | ||
icon { | ||
url | ||
} | ||
} | ||
} | ||
} | ||
`; | ||
|
||
export type IIndustriesQuery = { | ||
enterprise: { | ||
industries: Array<{ | ||
title: string; | ||
icon: { | ||
url: string; | ||
}; | ||
}>; | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import Quote from "@/components/Quote"; | ||
import { request } from "@/utils/graphQLClient"; | ||
|
||
import { | ||
lemonCashSectionQuery, | ||
lemonCashSectionQueryType, | ||
} from "../../queries/lemon-cash"; | ||
|
||
const LemonCashSection: React.FC = async () => { | ||
const quote = ( | ||
await request<lemonCashSectionQueryType>(lemonCashSectionQuery) | ||
).lemonCashSection; | ||
|
||
return <Quote className="lg:px-32" {...quote} />; | ||
}; | ||
|
||
export default LemonCashSection; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,80 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import clsx from "clsx"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import Card from "@/components/CtaCard"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import HighlightedText from "@/components/HighlightedText"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import Quote from "@/components/Quote"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { request } from "@/utils/graphQLClient"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
forGovernmentsQuery, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IForGovernmentsQuery, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getBlock, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ICCText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ICCCardsSection, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ICCLongText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ICCQuote, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ICCHightlightText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "./queries"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const ForGovernments: React.FC = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const sections = (await request<IForGovernmentsQuery>(forGovernmentsQuery)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.enterprise.GovernmentSection; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [{ fullText, highlightedText }] = getBlock<ICCHightlightText>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sections, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"ComponentContentHighlightText", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [{ longtext }] = getBlock<ICCLongText>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sections, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"ComponentContentLongText", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [{ text }] = getBlock<ICCText>(sections, "ComponentContentText"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [{ cards: objectivesCards }, { cards: disputeTypesCards }] = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getBlock<ICCCardsSection>(sections, "ComponentContentCardsSection"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [quote] = getBlock<ICCQuote>(sections, "ComponentContentQuote"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+19
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for GraphQL requests and data extraction The component currently doesn't include any error handling for the GraphQL request or for scenarios where expected blocks might be missing from the response. This could lead to runtime errors if the API returns unexpected data. Consider adding proper error handling: const ForGovernments: React.FC = async () => {
+ try {
const sections = (await request<IForGovernmentsQuery>(forGovernmentsQuery))
.enterprise.GovernmentSection;
+
+ // Check if we have the expected blocks
+ const highlightBlocks = getBlock<ICCHightlightText>(
+ sections,
+ "ComponentContentHighlightText",
+ );
+ if (highlightBlocks.length === 0) throw new Error("Missing highlight text block");
+ const [{ fullText, highlightedText }] = highlightBlocks;
- const [{ fullText, highlightedText }] = getBlock<ICCHightlightText>(
- sections,
- "ComponentContentHighlightText",
- );
// Similar checks for other blocks
// Rest of the component...
+ } catch (error) {
+ console.error("Error in ForGovernments component:", error);
+ return <div>Failed to load government section content. Please try again later.</div>;
+ }
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className={"flex flex-col gap-20 px-6 py-12 lg:gap-28 lg:px-32 lg:py-24"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className="space-y-6"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<HighlightedText | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{...{ fullText, highlightedText }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fullTextStyle="!text-primary-text !text-xl !font-medium lg:!text-2xl" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
highlightedTextStyle="!text-xl !font-medium lg:!text-2xl" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<p className="lg:text-lg">{longtext}</p> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className="grid grid-cols-1 gap-6 lg:grid-cols-3"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{objectivesCards.map((card) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<Card | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key={card.title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
title={card.title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
icon={card.icon} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
description={card.subtitle} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<Quote {...quote} /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<h3 className="mb-12 text-lg font-medium text-primary-text lg:text-xl"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{text} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</h3> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className="flex flex-wrap gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{disputeTypesCards.map((card) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
key={card.title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className={clsx( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"text-md rounded-2xl border border-stroke bg-background-2 p-6", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"text-primary-text lg:text-lg", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{card.title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export default ForGovernments; |
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.
🛠️ Refactor suggestion
Consider simplifying this component wrapper
This wrapper component doesn't add any functionality beyond passing props to the base
_HighlightedText
component. According to the retrieved learnings, the Kleros website prefers simple implementations that handle specific cases over complex solutions (YAGNI principle).There are a few issues to address:
The
IHighlightedText
type from the query only containsfullText
andhighlightedText
, but you're extending it with additional style properties. This suggests a mismatch between the type definition and its usage.The naming convention is confusing - typically, a component prefixed with
_
indicates a private/internal component, but here it's being wrapped by a public component with the same name.Consider one of these approaches:
HighlightedText
type in the query fileIf you decide to keep this wrapper, I suggest renaming the imported component to avoid confusion:
📝 Committable suggestion