-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/site refactor and fixes #69
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
WalkthroughThis pull request introduces a series of structural and visual changes across multiple frontend components. The primary modifications involve updating data structures, particularly the representation of URLs within objects, and making subtle adjustments to component layouts and styling. Changes span components related to reports, research development, and UI elements, with a consistent theme of nesting URL information within a Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 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: 0
🧹 Nitpick comments (1)
frontend/src/components/ResearchDevelopment/TabSection/FellowshipTabContent/Testimonials/TestimonialCard.tsx (1)
4-13
: Consider making the iframe dimensions more responsive.While the current fixed height works, consider using aspect-ratio for better responsiveness across different screen sizes.
- <div className="relative h-[203px] w-full overflow-hidden rounded-2xl"> + <div className="relative w-full overflow-hidden rounded-2xl aspect-video">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/src/app/for-lawyers/components/Flowchart.tsx
(2 hunks)frontend/src/app/home/components/LearnPosts.tsx
(1 hunks)frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx
(1 hunks)frontend/src/components/Cooperative/ReportSection/ReportCard.tsx
(1 hunks)frontend/src/components/CtaCard.tsx
(1 hunks)frontend/src/components/ExternalLink.tsx
(1 hunks)frontend/src/components/ResearchDevelopment/TabSection/FellowshipTabContent/Testimonials/TestimonialCard.tsx
(1 hunks)frontend/src/components/ResearchDevelopment/TabSection/ResearchTabContent/KlerosBook.tsx
(1 hunks)frontend/src/queries/cooperative/report-section.ts
(2 hunks)frontend/src/queries/research-development/tabs-data.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/app/home/components/LearnPosts.tsx
🧰 Additional context used
📓 Learnings (1)
frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-website-v2#37
File: frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx:65-71
Timestamp: 2025-01-07T09:32:38.688Z
Learning: In React's useEffect hook, dependencies that are guaranteed to remain static (not change between renders) can be safely omitted from the dependency array, even if the exhaustive-deps lint rule suggests including them. However, this should be done with caution and proper documentation to explain why the dependencies are intentionally omitted.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - kleros-website-v2
- GitHub Check: Header rules - kleros-website-v2
- GitHub Check: Pages changed - kleros-website-v2
🔇 Additional comments (12)
frontend/src/components/CtaCard.tsx (2)
48-50
: LGTM! Good spacing and responsive text styling.The margin and text styling provide good readability and consistent spacing in the layout.
52-52
: LGTM! Smart use of flexbox for dynamic content.Using
mt-auto
in the flex container ensures the arrow link section is consistently positioned at the bottom, regardless of content length variations. This creates a clean, uniform appearance across different cards.frontend/src/components/ExternalLink.tsx (1)
19-22
: LGTM! Improved layout using flex.The switch to flex layout with proper spacing improves the alignment and consistency of the link elements.
frontend/src/queries/cooperative/report-section.ts (2)
65-65
: LGTM! Type definitions match GraphQL schema.The TypeScript types have been properly updated to reflect the new nested structure.
Also applies to: 70-73, 76-79
35-37
: Verify the impact of GraphQL schema changes.The URL field has been moved into a nested
file
object. This is a breaking change that requires updates in all components consuming this data.Also applies to: 42-44, 50-52
✅ Verification successful
Schema changes are properly handled ✅
The codebase is already adapted to work with the nested
file.url
structure:
- Types are correctly defined in ReportCard component
- All components in the chain are properly typed
- No instances of direct URL access that need updating
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find components that might be affected by the schema change. # Search for direct url access patterns that need updating rg -l 'annualReports.*\.url|riskReports.*\.url|treasuryReports.*\.url' --type ts --type tsx # Search for potential GraphQL queries that might need updating rg -l 'gql`.*annualReports|riskReports|treasuryReports' --type tsLength of output: 300
Script:
#!/bin/bash # Check the component implementation for url access patterns cat frontend/src/components/Cooperative/ReportSection/index.tsxLength of output: 1165
Script:
#!/bin/bash # Check the ReportCard component implementation cat frontend/src/components/Cooperative/ReportSection/ReportCard.tsxLength of output: 1873
frontend/src/components/ResearchDevelopment/TabSection/ResearchTabContent/KlerosBook.tsx (1)
38-39
: LGTM! Updated to use new data structure.The component has been properly updated to use the new
file.url
structure and renamed properties.frontend/src/components/Cooperative/ReportSection/ReportCard.tsx (1)
15-15
: LGTM! Type definition change aligns with the refactoring pattern.The restructuring of the URL property into a nested file object is consistent with similar changes across other components.
frontend/src/app/for-lawyers/components/Flowchart.tsx (2)
35-35
: LGTM! Arrow positioning refinement.The
-left-[1px]
adjustment provides pixel-perfect alignment for the arrow indicator.
63-63
: LGTM! Improved layout centering.Centering the flowchart items on medium screens provides better visual balance.
frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx (1)
78-78
: LGTM! URL access updated to match new data structure.The change correctly accesses the URL through the new nested file object structure, maintaining consistency with the type changes in ReportCard.tsx.
frontend/src/queries/research-development/tabs-data.ts (2)
23-24
: LGTM! GraphQL query updated to match new structure.The query fields have been updated to match the new data structure with nested file objects.
143-146
: LGTM! Type definitions aligned with GraphQL schema.The KlerosBook type has been updated to reflect the new structure, maintaining type safety.
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
🧹 Nitpick comments (1)
frontend/src/components/Cooperative/ReportSection/ReportCard.tsx (1)
33-59
: Consider enhancing error handling and function organization.While the download functionality is well-implemented with proper cleanup and dynamic file extension handling, consider these improvements:
- Add user-friendly error feedback instead of just console.error
- Split the function into smaller, focused functions for better maintainability
Example refactor:
+ const createDownloadLink = (url: string, filename: string) => { + const link = document.createElement("a"); + link.href = url; + link.download = filename; + return link; + }; + + const getFileExtension = (contentType: string): string => { + const extension = contentType.split("/")[1] || "octet-stream"; + return extension === "json" ? "json" : extension; + }; + + const generateFileName = (report: Reports[number], fileExtension: string): string => { + return `${report.month ? report.month + "-" : ""}${report.year}.${fileExtension}`; + }; + const handleDownload = () => { if (!selectedReport || !selectedReport.file.url) return; fetch(selectedReport.file.url) .then((response) => response.blob()) .then((blob) => { const url = window.URL.createObjectURL(new Blob([blob])); - const link = document.createElement("a"); - link.href = url; - const contentType = blob.type; - - const extension = contentType.split("/")[1] || "octet-stream"; - const fileExtension = extension === "json" ? "json" : extension; - - link.download = `${selectedReport.month ? selectedReport.month + "-" : ""}${selectedReport.year}.${fileExtension}`; + const fileExtension = getFileExtension(contentType); + const fileName = generateFileName(selectedReport, fileExtension); + const link = createDownloadLink(url, fileName); document.body.appendChild(link); link.click(); document.body.removeChild(link); window.URL.revokeObjectURL(url); }) .catch((error) => { - console.error("Error fetching the file:", error); + // TODO: Add user-friendly error notification + console.error("Failed to download report:", error); }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx
(2 hunks)frontend/src/components/Cooperative/ReportSection/ReportCard.tsx
(3 hunks)frontend/src/components/ResearchDevelopment/TabSection/ResearchTabContent/KlerosBook.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/ResearchDevelopment/TabSection/ResearchTabContent/KlerosBook.tsx
- frontend/src/components/Cooperative/ReportSection/DropdownContainer.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - kleros-website-v2
- GitHub Check: Header rules - kleros-website-v2
- GitHub Check: Pages changed - kleros-website-v2
🔇 Additional comments (4)
frontend/src/components/Cooperative/ReportSection/ReportCard.tsx (4)
14-14
: LGTM! Type definition change improves data structure.The nested object structure for file URLs provides better organization and extensibility for future file metadata.
31-31
: LGTM! State management improvement.Storing the entire report object instead of just the URL provides better access to report metadata and maintains type safety.
77-77
: LGTM! Props update aligns with state management changes.The DropdownContainer component correctly receives the new setSelectedReport prop.
83-90
: LGTM! Button implementation is well-structured.The Button component:
- Correctly handles the disabled state
- Uses appropriate styling
- Properly integrates with the new download functionality
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
🧹 Nitpick comments (4)
frontend/src/components/ForBuilders/Hero.tsx (1)
48-50
: LGTM! Consider adding quality prop for optimization.The use of
fill
andpriority
props improves the background image's responsiveness and loading performance.Consider adding the
quality
prop to optimize image quality vs performance:<Image src={heroData.background.url} alt="Hero Image Background" fill priority + quality={85} className="absolute left-0 top-0 z-[-1] h-full object-cover" />
frontend/src/components/Navbar/AppsDropdownContent/Card.tsx (1)
28-39
: Consider extracting complex className logic.The className composition is getting complex with multiple conditions. Consider extracting this logic into a separate function or constant for better maintainability.
+const getCardClassName = (variant: "large" | "medium" | "small", className?: string) => + clsx( + cardBaseStyle, + hoverEffect, + "flex-row", + "w-full", + { + "xl:flex-col xl:pb-8": variant === "large", + "xl:flex-row": variant === "medium" || variant === "small", + "xl:w-[380px]": true, + }, + className, + ); const Card: React.FC<CardProps> = ({ solution, variant, className }) => { return ( <CustomLink key={solution?.solution_name} href={solution?.url} - className={clsx( - cardBaseStyle, - hoverEffect, - "flex-row", - "w-full", - { - "xl:flex-col xl:pb-8": variant === "large", - "xl:flex-row": variant === "medium" || variant === "small", - "xl:w-[380px]": true, - }, - className, - )} + className={getCardClassName(variant, className)} >frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/SafeSnap.tsx (2)
8-10
: Consider strengthening type safety.The interface could be more specific about the expected shape of
useCasesData.solutionSections
.interface ISafeSnap { useCasesData: { solutionSections: { title: string; description: string; solutionHeader: string; partnersHeader: string; solution: { solution_name: string; // other solution props }; partners: Array<{ name: string; url: string; icon_svg: { url: string; }; }>; }; }; }
42-60
: Consider memoizing partner links for performance.If the partners list is large, consider memoizing the partner links to prevent unnecessary re-renders.
+const PartnerLink = React.memo(({ partner }: { partner: typeof partners[number] }) => ( + <CustomLink + key={partner?.name} + href={partner?.url} + aria-label={`Visit ${partner?.name}'s website`} + className={clsx( + "transform transition duration-100 hover:scale-[1.01]", + "cursor-pointer", + )} + > + <Image + key={partner?.name} + src={partner?.icon_svg?.url} + alt={partner?.name} + width={64} + height={64} + aria-hidden="true" + /> + </CustomLink> +)); +PartnerLink.displayName = 'PartnerLink'; // In the main component {useCasesData.solutionSections.partners.map((partner) => ( - <CustomLink ... /> + <PartnerLink key={partner?.name} partner={partner} /> ))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
frontend/src/components/ForBuilders/Hero.tsx
(3 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/Header.tsx
(1 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/Header.tsx
(1 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/HowKlerosSolvesIt.tsx
(2 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/SafeSnap.tsx
(1 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/LearnMore.tsx
(0 hunks)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/index.tsx
(2 hunks)frontend/src/components/ForBuilders/UseCasesSection/index.tsx
(1 hunks)frontend/src/components/IntegrateSection/LearnMore.tsx
(0 hunks)frontend/src/components/IntegrateSection/index.tsx
(1 hunks)frontend/src/components/LearnMore.tsx
(2 hunks)frontend/src/components/Navbar/AppsDropdownContent/Card.tsx
(1 hunks)frontend/src/components/Navbar/AppsDropdownContent/index.tsx
(2 hunks)frontend/src/components/UseCasesCards.tsx
(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/src/components/IntegrateSection/LearnMore.tsx
- frontend/src/components/ForBuilders/UseCasesSection/DAOSection/LearnMore.tsx
✅ Files skipped from review due to trivial changes (2)
- frontend/src/components/UseCasesCards.tsx
- frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/Header.tsx
🔇 Additional comments (24)
frontend/src/components/ForBuilders/UseCasesSection/index.tsx (2)
14-14
: Great improvements to responsive padding!The padding changes enhance the responsive design by:
- Using the more concise
py-16
syntax- Adding appropriate spacing for medium screens with
md:px-32
andmd:py-24
15-15
: Excellent typography improvements!The heading style changes enhance readability across devices by:
- Using responsive text sizing (
text-xl
→lg:text-3xl
)- Adding appropriate font weight with
font-medium
- Implementing responsive margins (
mb-8
→lg:mb-12
)frontend/src/components/ForBuilders/UseCasesSection/DAOSection/Header.tsx (3)
13-13
: Great responsive margin adjustment!The change from a fixed margin to a responsive one (
mb-12 lg:mb-16
) improves the mobile-first design by using appropriate spacing across different screen sizes.
14-14
: Good typography scaling implementation!The heading now scales appropriately with screen size:
- Smaller text (text-lg) on mobile for better readability
- Larger text (text-2xl) on desktop for visual hierarchy
17-17
: Verify the large margin on desktop screens.While the responsive text sizing looks good, the large margin (
lg:mb-16
) between the paragraph and the image on desktop screens might create too much whitespace. Consider testing with a smaller margin likelg:mb-12
if the spacing appears too generous.frontend/src/components/ForBuilders/Hero.tsx (3)
17-17
: Verify the increased vertical spacing.The significant increase in padding (especially
pb-60
) might create excessive whitespace. Please verify that this spacing aligns with the design and doesn't create awkward gaps on different screen sizes.
19-21
: LGTM! Improved heading responsiveness.The heading changes enhance readability with better font sizing across breakpoints and improved visual hierarchy through the medium font weight.
39-39
: LGTM! Enhanced arrow icon visibility.The responsive sizing improves the arrow icon's visibility on desktop while maintaining appropriate proportions on mobile.
frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/HowKlerosSolvesIt.tsx (3)
5-5
: LGTM! Good job simplifying imports.The removal of unused imports and delegation to
SafeSnap
improves code maintainability.
14-16
: LGTM! Responsive styling improvements.The header styling now properly handles different screen sizes with the
lg
breakpoint.
17-17
: LGTM! Good component composition.Delegating rendering to
SafeSnap
improves component modularity and separation of concerns.frontend/src/components/ForBuilders/UseCasesSection/DAOSection/index.tsx (2)
3-3
: LGTM! Good component simplification.Replacing LearnMore with ExternalLink reduces complexity while maintaining functionality.
18-22
: LGTM! Clean ExternalLink implementation with responsive styling.The ExternalLink props are properly derived from useCasesData and the styling classes handle different screen sizes well.
frontend/src/components/LearnMore.tsx (4)
3-3
: LGTM! Good use of clsx.Using clsx for class name composition is a best practice.
17-17
: LGTM! Good interface extension.Making className optional maintains backward compatibility while adding flexibility.
20-25
: LGTM! Clean props destructuring.Props are properly typed and destructured.
28-31
: LGTM! Proper class composition.Using clsx to combine base classes with optional className prop is well implemented.
frontend/src/components/IntegrateSection/index.tsx (5)
8-8
: LGTM! Component replacement.Replacing CtaBox with LearnMore aligns with the refactoring objectives.
16-16
: LGTM! Improved container responsiveness.The padding classes properly handle different screen sizes.
20-24
: LGTM! Better visual hierarchy.The text size and margin adjustments improve readability across screen sizes.
27-31
: LGTM! Clean LearnMore implementation.Props are properly passed and the className override is well handled.
34-34
: LGTM! Responsive link styling.The ExternalLink styling properly handles text alignment and margins across screen sizes.
frontend/src/components/Navbar/AppsDropdownContent/index.tsx (1)
3-3
: LGTM! Clean refactor to ExternalLink component.The change from CustomLink to ExternalLink is consistent with the PR's refactoring objectives. The implementation is clean and maintains the same functionality.
Also applies to: 52-56
frontend/src/components/Navbar/AppsDropdownContent/Card.tsx (1)
20-20
: LGTM! Good addition of className prop.The optional className prop addition improves component reusability by allowing style customization.
Also applies to: 23-23
frontend/src/components/ForBuilders/UseCasesSection/DAOSection/KeyChallenges/SafeSnap.tsx
Show resolved
Hide resolved
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.
lgtm
Summary by CodeRabbit
Style
Refactor
file
object withurl
.New Features
TestimonialCard
to embed YouTube videos instead of displaying thumbnails.SafeSnap
component for rendering use case data.Bug Fixes