Refactor(frontend): Navbar and animations #75
Conversation
WalkthroughThis pull request introduces a new dependency for the motion library and implements various updates across multiple frontend components. Changes include modifications to class name ordering, integration of the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NB as Navbar
participant MB as MobileMenu
U->>NB: Click hamburger button
NB->>NB: Toggle menu state with useToggle
NB->>MB: Render MobileMenu with motion animation
U->>MB: Click a navigation link
MB->>NB: Invoke closeFn to close the menu
sequenceDiagram
participant U as User
participant DN as DesktopNavigation
participant DD as Dropdown (Apps/Resources)
U->>DN: Click desktop nav link
DN->>DD: Render dropdown using motion.div & AnimatePresence
U->>DD: Click a link within the dropdown
DD->>DN: Trigger closeFn to collapse the dropdown
Possibly related PRs
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
🪧 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
frontend/src/app/layout.tsx (1)
29-29: Remove redundantclsxusage or add conditional classes.The
clsxutility is used with a single class name, making it redundant. Either:
- Remove
clsxand useurbanist.classNamedirectly, or- Add conditional classes to justify the use of
clsx.- <main className={clsx(urbanist.className)}> + <main className={urbanist.className}>frontend/src/components/Navbar/AppsDropdownContent/index.tsx (1)
18-23: Enhance grid layout accessibility.The grid layout should be enhanced with ARIA attributes for better screen reader support:
- Add
role="grid"to the grid container- Add
role="gridcell"to grid items<div className={clsx( "grid w-full grid-cols-1 gap-3 bg-background-2 lg:grid-flow-col", "lg:grid-cols-3 lg:grid-rows-4 lg:gap-4", - )} + )} + role="grid"frontend/src/components/Navbar/AppsDropdownContent/Card.tsx (1)
30-35: Add descriptive ARIA attributes.The
Imagecomponent should have descriptive ARIA attributes for better accessibility.<Image src={solution?.logo_svg?.url} alt={solution?.solution_name} width={64} height={64} + role="img" + aria-label={`${solution?.solution_name} logo`}frontend/src/components/Navbar/HamburgerButton.tsx (1)
46-69: Consider adding ARIA attributes for accessibility.The hamburger button should be properly labeled for screen readers.
const HamburgerButton: React.FC<IHamburgerButton> = ({ className, isOpen }) => { return ( - <div className={clsx("relative", className)}> + <div + className={clsx("relative", className)} + role="button" + aria-label="Toggle menu" + aria-expanded={isOpen} + >frontend/src/components/Navbar/ResourcesDropdownContent/index.tsx (1)
38-50: Consider extracting link rendering to a separate component.The link rendering logic could be extracted to improve readability and reusability.
+const ResourceLink: React.FC<{ link: { url: string; name: string }, closeFn: () => void }> = ({ link, closeFn }) => ( + <li className="w-max transform transition duration-75 hover:scale-[1.01]"> + <CustomLink href={link.url} onClick={closeFn}> + {link.name} + </CustomLink> + </li> +); <ul className="flex flex-col gap-2"> - {section.links?.map((link) => ( - <li - key={link.url} - className={"w-max transform transition duration-75 hover:scale-[1.01]"} - > - <CustomLink href={link.url} onClick={closeFn}> - {link.name} - </CustomLink> - </li> - ))} + {section.links?.map((link) => ( + <ResourceLink key={link.url} link={link} closeFn={closeFn} /> + ))} </ul>frontend/src/app/home/components/TrustedBy.tsx (1)
62-65: Consider extracting animation classes to Tailwind components.The hover and animation classes could be extracted to Tailwind components for better reuse.
// tailwind.config.js module.exports = { theme: { extend: { + components: { + 'partner-logo': 'relative mx-2 inline-block h-16 w-16 rounded-full bg-white hover:cursor-pointer lg:mx-10', + } } } } // Component className={clsx("partner-logo")}frontend/src/components/Navbar/MobileMenu.tsx (1)
51-58: Consider extracting class names to a constant.The class names could be extracted to a constant for better maintainability and reusability.
+const containerClasses = [ + "z-50 w-screen overflow-y-auto rounded-b-lg", + "bg-background-2 p-6 shadow-lg", +] as const; return ( <div className={clsx( className, - "z-50 w-screen overflow-y-auto rounded-b-lg", - "bg-background-2 p-6 shadow-lg", + ...containerClasses, )} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
frontend/package.json(1 hunks)frontend/src/app/home/components/TrustedBy.tsx(1 hunks)frontend/src/app/layout.tsx(2 hunks)frontend/src/components/Footer.tsx(2 hunks)frontend/src/components/Navbar/AppsDropdownContent/Card.tsx(1 hunks)frontend/src/components/Navbar/AppsDropdownContent/index.tsx(1 hunks)frontend/src/components/Navbar/DesktopNavigation.tsx(3 hunks)frontend/src/components/Navbar/HamburgerButton.tsx(1 hunks)frontend/src/components/Navbar/MobileMenu.tsx(5 hunks)frontend/src/components/Navbar/ResourcesDropdownContent/index.tsx(2 hunks)frontend/src/components/Navbar/index.tsx(3 hunks)frontend/src/components/OverlayScrollbarMain.tsx(1 hunks)frontend/src/styles/globals.css(0 hunks)frontend/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/styles/globals.css
🔇 Additional comments (10)
frontend/tailwind.config.ts (1)
33-33: LGTM! Color addition follows conventions.The new
background-darkcolor is semantically named and aligns with the existing color palette.frontend/src/components/Footer.tsx (2)
24-30: LGTM! Responsive grid layout enhances the footer structure.The implementation:
- Uses grid layout for better section organization
- Handles mobile (2x2) and desktop (4x1) layouts appropriately
- Includes proper spacing with gap utility
33-33: LGTM! Color changes improve visual hierarchy.The color updates for text and horizontal rule enhance the visual consistency of the footer.
Also applies to: 46-46
frontend/src/components/OverlayScrollbarMain.tsx (1)
21-24: Verify the impact of visible overflow on dropdowns and modals.Setting
overflow.yto"visible"allows content to overflow vertically. While this might be intentional for dropdowns or modals, verify that it doesn't cause unintended side effects like:
- Unwanted scrollbars
- Layout shifts
- Z-index stacking issues with overlapping content
Consider using a stacking context manager (e.g., Radix UI's Portal) for dropdowns and modals instead of relying on overflow visibility.
frontend/src/components/Navbar/HamburgerButton.tsx (1)
13-44: LGTM! Well-structured animation variants.The animation variants are well-organized with consistent durations and clear keyframe sequences.
frontend/src/components/Navbar/ResourcesDropdownContent/index.tsx (1)
7-11: LGTM! Well-defined props interface.The addition of
closeFnprop improves component reusability and control.frontend/src/components/Navbar/index.tsx (1)
79-105: LGTM! Well-implemented mobile menu animations.The use of AnimatePresence and motion.div provides smooth transitions with good initial and exit animations.
frontend/src/components/Navbar/MobileMenu.tsx (1)
22-31: LGTM! Interface changes follow best practices.The interface renaming and addition of new props enhance component flexibility and follow TypeScript naming conventions.
frontend/src/components/Navbar/DesktopNavigation.tsx (1)
82-115: LGTM! Well-implemented animations and interactions.The dropdown animations and interactions are well-implemented with proper:
- Use of AnimatePresence for mount/unmount animations
- Event propagation handling
- Smooth transition values
frontend/package.json (1)
18-18: Verify the motion library version.Let's check if there's a newer version of the motion library available and if there are any breaking changes or security issues to consider.
✅ Verification successful
Motion library dependency appears to be up-to-date.
The web query revealed several “motion” libraries across different platforms, none of which are directly related to the npm package used in our frontend project. The version we’re using (^12.0.11) aligns with the latest release for the JavaScript animation library and does not indicate any obvious breaking changes or vulnerabilities that require updating.
- The versions found in the web results (e.g., Apple Motion 5.9, Rokoko 3.0.0, Zaber 4.5.1, OMPL 1.6.0) refer to entirely different products.
- For our npm dependency, version 12.0.11 is current and no newer release has been confirmed for the same library.
🏁 Scripts executed
The following scripts were executed for the analysis:
Web query:
What is the latest version of the motion library?Length of output: 3231
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/src/components/Navbar/AppsDropdownContent/index.tsx (1)
50-55:⚠️ Potential issueFix
getRowSpanfunction's undefined return.The function doesn't handle indices >= 3, implicitly returning
undefined. This could lead to inconsistent layout behavior.const getRowSpan = ( index: number, ): "lg:row-span-4" | "lg:row-span-2" | undefined => { if (index === 0) return "lg:row-span-4"; if (index < 3) return "lg:row-span-2"; + return undefined; };frontend/src/components/Navbar/index.tsx (1)
4-4:⚠️ Potential issueUpdate import path for motion library.
Similar to HamburgerButton, the import path should be from framer-motion.
-import { motion, AnimatePresence } from "motion/react"; +import { motion, AnimatePresence } from "framer-motion";
🧹 Nitpick comments (2)
frontend/src/components/Navbar/MobileMenu.tsx (1)
91-111: Optimize dropdown animations for better performance.Consider adding
initial={false}to theAnimatePresencecomponent to skip initial animations and improve performance. Also, use CSS transform instead of height animation for better performance.-<AnimatePresence> +<AnimatePresence initial={false}> {openDropdownIndex === index ? ( <motion.div - initial={{ height: 0 }} - animate={{ height: "auto" }} - exit={{ height: 0 }} + initial={{ transform: 'scaleY(0)', originY: 0 }} + animate={{ transform: 'scaleY(1)' }} + exit={{ transform: 'scaleY(0)' }} + transition={{ duration: 0.2 }} className="mt-2 max-h-[30dvh] overflow-y-scroll pl-2 pr-4" >frontend/src/components/Navbar/DesktopNavigation.tsx (1)
82-120: Consider extracting animation values into constants.The animation implementation with
AnimatePresenceandmotion.divlooks great, but the animation values could be extracted into constants for better maintainability.Consider applying this refactor:
+const DROPDOWN_ANIMATION = { + backdrop: { + initial: { opacity: 0 }, + animate: { opacity: 1 }, + exit: { opacity: 0 }, + }, + content: { + initial: { translateY: "-5%" }, + animate: { translateY: 0 }, + exit: { translateY: "-5%" }, + }, +}; // Then in the JSX: <motion.div className={clsx( "fixed inset-0 top-20 z-40 h-[calc(100dvh-5rem)]", "bg-black/50", )} - initial={{ opacity: 0 }} - animate={{ opacity: 1 }} - exit={{ opacity: 0 }} + {...DROPDOWN_ANIMATION.backdrop} onClick={() => setOpenDropdownIndex(null)} > <motion.div className={clsx( "absolute top-0 max-h-full w-full overflow-y-auto", "bg-background-2", )} - initial={{ translateY: "-5%" }} - animate={{ translateY: 0 }} - exit={{ translateY: "-5%" }} + {...DROPDOWN_ANIMATION.content} onClick={(e) => e.stopPropagation()} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/components/IntegrateSection/index.tsx(1 hunks)frontend/src/components/Navbar/AppsDropdownContent/index.tsx(1 hunks)frontend/src/components/Navbar/DesktopNavigation.tsx(3 hunks)frontend/src/components/Navbar/MobileMenu.tsx(4 hunks)frontend/src/components/Navbar/index.tsx(3 hunks)frontend/src/styles/globals.css(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/styles/globals.css
⏰ 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 (5)
frontend/src/components/IntegrateSection/index.tsx (1)
26-29: LGTM! Styling adjustments look good.The changes to
classNameprops for bothAppsDropdownContentandLearnMorecomponents maintain consistent spacing and responsive design.Also applies to: 34-34
frontend/src/components/Navbar/index.tsx (1)
21-33: Add keyboard navigation support.The mobile menu should be accessible via keyboard and handle Escape key for closing.
+import { useEffect } from 'react'; const Navbar: React.FC<INavbar> = ({ navbarData }) => { + useEffect(() => { + const handleEscape = (e: KeyboardEvent) => { + if (e.key === 'Escape' && menuOpen) { + toggleMenuOpen(false); + } + }; + document.addEventListener('keydown', handleEscape); + return () => document.removeEventListener('keydown', handleEscape); + }, [menuOpen, toggleMenuOpen]); return ( // ... existing code ); };frontend/src/components/Navbar/DesktopNavigation.tsx (3)
56-56: Good UX improvement!Adding the dropdown close handler on link click prevents dropdowns from staying open after navigation.
76-78: Nice touch with the arrow rotation animation!The dynamic rotation class using
clsxprovides smooth feedback for the dropdown state.
108-108: Good component communication pattern!Passing the
closeFnprop to dropdown content components allows them to control their parent's state when needed, following React's unidirectional data flow.Also applies to: 114-114
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/components/Navbar/index.tsx (1)
56-61: Add ARIA attributes for better accessibility.While the implementation is good, the button should have ARIA attributes for better accessibility.
<button className="ml-auto block text-white lg:hidden" onClick={toggleMenuOpen} + aria-label="Toggle mobile menu" + aria-expanded={menuOpen} + aria-controls="mobile-menu" > <HamburgerButton className="h-5 w-6" isOpen={menuOpen} /> </button>frontend/src/components/Navbar/MobileMenu.tsx (1)
92-113: Consider using transform instead of height for animations.While the animations look good, animating height can cause layout shifts and impact performance. Consider using transform instead.
<AnimatePresence> {openDropdownIndex === index ? ( <motion.div - initial={{ height: 0 }} - animate={{ height: "auto" }} - exit={{ height: 0 }} + initial={{ transform: "scaleY(0)", originY: 0 }} + animate={{ transform: "scaleY(1)" }} + exit={{ transform: "scaleY(0)" }} className="mt-2 max-h-[30dvh] overflow-y-scroll pl-2 pr-4" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Navbar/MobileMenu.tsx(4 hunks)frontend/src/components/Navbar/index.tsx(3 hunks)
⏰ 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 (7)
frontend/src/components/Navbar/index.tsx (4)
4-4: Update import path for motion library.The import path should be from framer-motion.
-import { motion, AnimatePresence } from "motion/react"; +import { motion, AnimatePresence } from "framer-motion";
22-22: LGTM! Good use of specialized hook.Using
useTogglefrom react-use simplifies the state management and provides better semantics for boolean state.
43-54: LGTM! Good UX improvement.The logo link now properly closes the mobile menu when clicked, improving user experience.
77-110: Add keyboard navigation support.The mobile menu should be accessible via keyboard and handle Escape key for closing.
+import { useEffect } from 'react'; const Navbar: React.FC<INavbar> = ({ navbarData }) => { + useEffect(() => { + const handleEscape = (e: KeyboardEvent) => { + if (e.key === 'Escape' && menuOpen) { + toggleMenuOpen(false); + } + }; + document.addEventListener('keydown', handleEscape); + return () => document.removeEventListener('keydown', handleEscape); + }, [menuOpen, toggleMenuOpen]); return ( // ... existing code ); };frontend/src/components/Navbar/MobileMenu.tsx (3)
23-31: LGTM! Good type safety improvement.Extending
React.ComponentProps<"div">improves type safety and component flexibility by allowing HTML div props to be passed through.
33-43: LGTM! Good props handling.The component properly handles the new props and correctly spreads remaining props to the root element.
100-109: LGTM! Good prop passing.The dropdown content components properly receive the closeFn prop for consistent menu closing behavior.
Summary by CodeRabbit
New Features
closeFnprop to several components for better dropdown management.background-darkto the Tailwind CSS configuration.Style
AppsDropdownContentandLearnMorecomponents.TrustedBycomponent for improved user experience.