-
Notifications
You must be signed in to change notification settings - Fork 88
feat: Introduce example page #30
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an example gallery and server-rendered detail pages, a registry to load example code/components, new client UI for preview/code with animation toggles, a branded sidebar, 11 new forwardRef animated SVG icons plus shared icon types, refactors one existing icon to the new pattern, and updates navbar links. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router as Next.js Router
participant Server as ExampleDetailPage (server)
participant Lib as lib/examples.ts
participant Detail as ExampleDetail (client)
participant UI as Preview/Code UI
User->>Router: Navigate to /example or /example/{slug}
Router->>Server: Request server page (if /example/{slug})
Server->>Lib: getExampleBySlug(slug)
Lib-->>Server: example object (component, code, metadata)
alt example found
Server->>Detail: render ExampleDetail with example prop
Detail->>UI: mount Preview/Code tabs + animation toggle
User->>UI: interact (toggle tabs / animation / copy)
UI-->>User: show live preview or code viewer, copy feedback
else not found
Server->>Router: call notFound()
Router-->>User: 404
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
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: 10
🧹 Nitpick comments (9)
components/ui/filled-bell-icon.tsx (1)
54-66: UnusedstrokeWidthprop.The
strokeWidthprop is destructured (line 12) and passed to the SVG (line 62), but since this is a filled icon without stroke styling, it has no visible effect. Consider either removingstrokeWidthfrom the accepted props or documenting that it's ignored for filled icons.🔎 Option: Remove unused strokeWidth
const FilledBellIcon = forwardRef<FilledBellIconHandle, AnimatedIconProps>( ( - { size = 24, color = "currentColor", strokeWidth = 2, className = "" }, + { size = 24, color = "currentColor", className = "" }, ref, ) => {viewBox="0 0 24 24" - strokeWidth={strokeWidth} fill={color}app/example/page.tsx (1)
38-40: Consider passingisAnimated={false}for preview consistency.The
ExampleDetailcomponent (in component-wrapper.tsx) passesisAnimatedtoexample.component, but the preview here renders without it. If example components expect this prop, consider explicitly passingisAnimated={false}to disable animations in the gallery preview cards.🔎 Proposed change
<div className="pointer-events-none origin-center scale-[0.6] select-none"> - <example.component /> + <example.component isAnimated={false} /> </div>app/example/[slug]/page.tsx (1)
63-70: Hardcoded platform tags.The "React" and "TypeScript" tags are hardcoded. Consider making these dynamic based on example metadata if different platforms may be supported in the future, or keep as-is if all examples are guaranteed to be React/TypeScript.
components/ui/dots-horizontal-icon.tsx (1)
5-8: Consider using the sharedAnimatedIconHandletype for consistency.
GithubIconimports and usesAnimatedIconHandlefrom./types, while this component defines its ownDotsHorizontalIconHandle. For consistency across the icon library, consider using the shared type.🔎 Proposed fix
-import { AnimatedIconProps } from "./types"; +import { AnimatedIconProps, AnimatedIconHandle } from "./types"; import { motion, useAnimate } from "motion/react"; -export type DotsHorizontalIconHandle = { - startAnimation: () => void; - stopAnimation: () => void; -}; - -const DotsHorizontalIcon = forwardRef< - DotsHorizontalIconHandle, +const DotsHorizontalIcon = forwardRef< + AnimatedIconHandle, AnimatedIconProps >(components/ui/unordered-list-icon.tsx (1)
70-114: Unnecessaryasynckeyword onstopfunction.The
stopfunction is markedasyncbut doesn't useawait. Consider removing theasynckeyword for consistency.🔎 Proposed fix
- const stop = useCallback(async () => { + const stop = useCallback(() => {components/ui/magnifier-icon.tsx (2)
17-35: Consider wrappingstartandstopwithuseCallbackfor consistency.Other icon components in this PR (
UsersIcon,TwitterXIcon) wrap their animation functions withuseCallback. For consistency and stable function references inuseImperativeHandle, consider doing the same here.🔎 Suggested change
+import { forwardRef, useImperativeHandle, useCallback } from "react"; -import { forwardRef, useImperativeHandle } from "react"; import { AnimatedIconProps } from "./types"; import { motion, useAnimate } from "motion/react"; // ... - const start = async () => { + const start = useCallback(async () => { await animate( ".magnifier-group", { x: [0, 1, 0, -1, 0], y: [0, -1, -2, -1, 0], rotate: [0, -5, 5, -5, 0], }, { duration: 1, ease: "easeInOut" }, ); - }; + }, [animate]); - const stop = () => { + const stop = useCallback(() => { animate( ".magnifier-group", { x: 0, y: 0, rotate: 0 }, { duration: 0.2, ease: "easeOut" }, ); - }; + }, [animate]);
42-48: Redundant wrapper functions.
handleHoverStartandhandleHoverEndsimply delegate tostartandstop. You can passstartandstopdirectly toonHoverStartandonHoverEnd.🔎 Suggested simplification
- const handleHoverStart = () => { - start(); - }; - - const handleHoverEnd = () => { - stop(); - }; - return ( <motion.svg ref={scope} - onHoverStart={handleHoverStart} - onHoverEnd={handleHoverEnd} + onHoverStart={start} + onHoverEnd={stop}lib/examples.ts (1)
39-42: Consider optimizing for larger registries.
getExampleBySlugreads all example files to find one. For a small registry this is fine, but as it grows, consider finding the registry entry first and reading only that file.🔎 Optimized implementation
export function getExampleBySlug(slug: string) { - const examples = getExamples(); - return examples.find((example) => example.slug === slug); + const registryEntry = EXAMPLE_REGISTRY.find((e) => e.slug === slug); + if (!registryEntry) return undefined; + + let code = ""; + try { + code = fs.readFileSync( + path.join(process.cwd(), registryEntry.filePath), + "utf8", + ); + } catch (error) { + console.error(`Failed to read example file: ${registryEntry.filePath}`, error); + code = "// Error: Could not load source code"; + } + + return { + componentName: registryEntry.componentName, + slug: registryEntry.slug, + createdBy: registryEntry.createdBy, + description: registryEntry.description, + tags: registryEntry.tags, + code, + component: registryEntry.component, + }; }components/ui/users-icon.tsx (1)
17-43: Unnecessaryasynckeyword onstart.The
startfunction doesn'tawaittheanimatecalls (they run in parallel intentionally), so theasynckeyword is unnecessary. Same applies tostopon line 45.🔎 Suggested cleanup
- const start = useCallback(async () => { + const start = useCallback(() => { // Primary user bounces up slightly animate(- const stop = useCallback(async () => { + const stop = useCallback(() => { animate(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
app/example/[slug]/page.tsxapp/example/page.tsxcomponents/examples/component-wrapper.tsxcomponents/examples/x-sidebar.tsxcomponents/navbar.tsxcomponents/ui/bookmark-icon.tsxcomponents/ui/dots-horizontal-icon.tsxcomponents/ui/filled-bell-icon.tsxcomponents/ui/github-icon.tsxcomponents/ui/home-icon.tsxcomponents/ui/magnifier-icon.tsxcomponents/ui/mail-filled-icon.tsxcomponents/ui/twitter-x-icon.tsxcomponents/ui/types.tscomponents/ui/unordered-list-icon.tsxcomponents/ui/user-check-icon.tsxcomponents/ui/users-icon.tsxlib/examples.tspublic/r/home-icon.json
🧰 Additional context used
🧬 Code graph analysis (14)
app/example/page.tsx (2)
lib/examples.ts (1)
getExamples(20-37)components/examples/component-wrapper.tsx (1)
Example(9-18)
components/ui/mail-filled-icon.tsx (2)
components/ui/types.ts (1)
AnimatedIconProps(1-10)icons/mail-filled-icon.tsx (1)
ref(6-66)
components/ui/unordered-list-icon.tsx (2)
components/ui/types.ts (1)
AnimatedIconProps(1-10)icons/unordered-list-icon.tsx (1)
ref(6-156)
components/ui/home-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/ui/dots-horizontal-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/ui/filled-bell-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/examples/component-wrapper.tsx (2)
components/ui/tabs.tsx (4)
Tabs(64-64)TabsList(64-64)TabsTrigger(64-64)TabsContent(64-64)lib/utils.ts (1)
cn(4-6)
components/ui/twitter-x-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/ui/bookmark-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
app/example/[slug]/page.tsx (2)
lib/examples.ts (1)
getExampleBySlug(39-42)components/examples/component-wrapper.tsx (1)
ExampleDetail(48-93)
components/ui/users-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/examples/x-sidebar.tsx (1)
components/ui/types.ts (2)
AnimatedIconProps(1-10)AnimatedIconHandle(12-15)
components/ui/user-check-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
components/ui/magnifier-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-10)
🔇 Additional comments (20)
components/ui/types.ts (1)
1-15: LGTM!Clean and well-documented type definitions for the animated icon system. The separation of props and imperative handle interfaces follows React best practices.
public/r/home-icon.json (1)
1-20: LGTM!The registry JSON follows the shadcn schema correctly and the embedded component code is consistent with the animated icon pattern used elsewhere in the PR.
components/ui/filled-bell-icon.tsx (1)
17-39: LGTM on animation logic.The bell ringing animation with staggered rotation keyframes creates a nice visual effect. Good use of
useCallbackfor memoization.app/example/page.tsx (2)
53-56: Potential runtime error ifcreatedBydoesn't contain a/.If
createdByis a string without/,.split("/").pop()returns the entire string, which is fine. However, ifcreatedByis an empty string, this would also return an empty string. The current implementation is safe.
6-27: LGTM on page structure.Clean implementation of the gallery page with responsive grid layout. Good use of Server Components for data fetching.
components/ui/mail-filled-icon.tsx (1)
1-75: LGTM!The implementation follows the established animated icon pattern consistently. The 3D rotation animation with
perspective-distantcreates a nice mail-opening effect.app/example/[slug]/page.tsx (2)
8-16: LGTM on Next.js 15 async params handling.Correctly typed
paramsasPromiseand properly awaited, which aligns with Next.js 15's async request APIs.
14-20: LGTM on page implementation.Good use of
notFound()for missing examples and clean integration withExampleDetailcomponent. The two-column layout with sidebar is well-structured.Also applies to: 53-54
components/ui/github-icon.tsx (1)
1-70: LGTM!The component follows the established pattern for animated icons with proper forwardRef usage, imperative handle exposure, and hover-triggered animations. Good use of the shared
AnimatedIconHandletype fromtypes.tsfor consistency.components/ui/bookmark-icon.tsx (1)
10-91: LGTM!The component correctly implements the animated icon pattern with forwardRef and imperative handle. The use of a
motion.divwrapper around the SVG is appropriate here since the animation targets an internal path element.components/ui/home-icon.tsx (1)
10-78: LGTM!Well-structured animation sequence with the roof, house, and door elements animating in a logical order. Good use of
useCallbackfor memoization and propertransformOriginsettings.components/ui/user-check-icon.tsx (1)
10-116: LGTM!Nice animation implementation with the user avatar bounce and checkmark path drawing effect using
pathLength. Theinitial={{ pathLength: 1 }}properly sets the checkmark as visible by default.components/examples/x-sidebar.tsx (1)
82-105: LGTM on the overall sidebar structure.Clean component composition with proper prop passing for the
isAnimatedtoggle. The responsive width classes and styling are appropriate.components/ui/unordered-list-icon.tsx (1)
116-168: LGTM!The animation implementation is well done with staggered line extensions and bullet pulse effects. Proper
transformOriginsettings ensure animations scale from the correct anchor points.components/ui/magnifier-icon.tsx (1)
50-83: LGTM!The SVG structure, animation target setup with
transformOrigin, and component export are correctly implemented.lib/examples.ts (1)
7-18: LGTM - Registry structure is well-defined.The registry entry structure is clean and extensible. Minor note: the tag "Framer Motion" could be updated to "Motion" to reflect the library's rebrand, but this is purely cosmetic.
components/ui/users-icon.tsx (1)
76-108: LGTM!The SVG structure with separate animation groups for primary and secondary users is well-organized. The component follows the established pattern correctly.
components/ui/twitter-x-icon.tsx (1)
1-65: LGTM!The component correctly implements the animated icon pattern with proper
useCallbackmemoization, imperative handle exposure, and consistent SVG structure. The animation sequence (scale + rotate wiggle) is a nice touch for the X icon.components/examples/component-wrapper.tsx (2)
9-18: LGTM - Well-defined interface.The
Exampleinterface properly captures all the metadata needed for the example system, with appropriate optional fields.
48-93: Good implementation of the tabbed preview/code viewer.The
ExampleDetailcomponent cleanly separates preview and code views with proper tab integration. The animation toggle state management is straightforward and effective.
| const copyToClipboard = async () => { | ||
| await navigator.clipboard.writeText(code); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }; |
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.
Add error handling for clipboard API.
navigator.clipboard.writeText can fail due to permissions or browser support. Consider wrapping in try-catch to prevent unhandled promise rejection.
🔎 Suggested fix
const copyToClipboard = async () => {
- await navigator.clipboard.writeText(code);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ try {
+ await navigator.clipboard.writeText(code);
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ } catch (error) {
+ console.error("Failed to copy to clipboard:", error);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const copyToClipboard = async () => { | |
| await navigator.clipboard.writeText(code); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| }; | |
| const copyToClipboard = async () => { | |
| try { | |
| await navigator.clipboard.writeText(code); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| } catch (error) { | |
| console.error("Failed to copy to clipboard:", error); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In components/examples/component-wrapper.tsx around lines 23 to 27, the
clipboard write call is unprotected and can reject; wrap the await
navigator.clipboard.writeText(code) in a try-catch, optionally guard with if
(navigator.clipboard) for older browsers, and in the catch setCopied(false) (or
a failure state), and log or surface the error (console.error or user-facing
toast) so the promise rejection is handled and failures are visible to the user.
| const handleMouseEnter = () => { | ||
| if (isAnimated) { | ||
| ref.current?.startAnimation(); | ||
| } | ||
| }; | ||
|
|
||
| const handleMouseLeave = () => { | ||
| if (isAnimated) { | ||
| ref.current?.stopAnimation(); | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (!isAnimated) { | ||
| ref.current?.stopAnimation(); | ||
| } | ||
| }, [isAnimated]); | ||
|
|
||
| return ( | ||
| <Link | ||
| href={href} | ||
| className="group flex w-fit items-center gap-4 rounded-full px-4 py-3 text-xl transition-colors hover:bg-white/10" | ||
| onMouseEnter={handleMouseEnter} | ||
| onMouseLeave={handleMouseLeave} | ||
| > | ||
| <Icon className="h-7 w-7" ref={ref} /> | ||
| <span className="font-normal">{label}</span> | ||
| </Link> | ||
| ); | ||
| }; |
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.
Potential double animation trigger on hover.
The SidebarItem component triggers animations via handleMouseEnter/handleMouseLeave calling the ref's imperative methods, but the icon components themselves also have internal onHoverStart/onHoverEnd handlers. This causes both animation paths to fire simultaneously when hovering, potentially causing animation conflicts or double triggers.
Consider either:
- Removing the internal hover handlers from icon components and relying solely on the imperative API
- Or removing the mouse handlers here and letting icons manage their own hover state
🔎 Option 1: Disable icon's internal hover when controlled externally
The icon components could accept a disableHover prop to prevent internal hover handling when being controlled externally. Alternatively, only use the imperative API when isAnimated is controlled:
return (
<Link
href={href}
className="group flex w-fit items-center gap-4 rounded-full px-4 py-3 text-xl transition-colors hover:bg-white/10"
- onMouseEnter={handleMouseEnter}
- onMouseLeave={handleMouseLeave}
>
<Icon className="h-7 w-7" ref={ref} />
<span className="font-normal">{label}</span>
</Link>
);This lets the icons handle their own hover animations naturally.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleMouseEnter = () => { | |
| if (isAnimated) { | |
| ref.current?.startAnimation(); | |
| } | |
| }; | |
| const handleMouseLeave = () => { | |
| if (isAnimated) { | |
| ref.current?.stopAnimation(); | |
| } | |
| }; | |
| useEffect(() => { | |
| if (!isAnimated) { | |
| ref.current?.stopAnimation(); | |
| } | |
| }, [isAnimated]); | |
| return ( | |
| <Link | |
| href={href} | |
| className="group flex w-fit items-center gap-4 rounded-full px-4 py-3 text-xl transition-colors hover:bg-white/10" | |
| onMouseEnter={handleMouseEnter} | |
| onMouseLeave={handleMouseLeave} | |
| > | |
| <Icon className="h-7 w-7" ref={ref} /> | |
| <span className="font-normal">{label}</span> | |
| </Link> | |
| ); | |
| }; | |
| const handleMouseEnter = () => { | |
| if (isAnimated) { | |
| ref.current?.startAnimation(); | |
| } | |
| }; | |
| const handleMouseLeave = () => { | |
| if (isAnimated) { | |
| ref.current?.stopAnimation(); | |
| } | |
| }; | |
| useEffect(() => { | |
| if (!isAnimated) { | |
| ref.current?.stopAnimation(); | |
| } | |
| }, [isAnimated]); | |
| return ( | |
| <Link | |
| href={href} | |
| className="group flex w-fit items-center gap-4 rounded-full px-4 py-3 text-xl transition-colors hover:bg-white/10" | |
| > | |
| <Icon className="h-7 w-7" ref={ref} /> | |
| <span className="font-normal">{label}</span> | |
| </Link> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In components/examples/x-sidebar.tsx around lines 47 to 76, hovering causes
duplicate animation triggers because both the sidebar handlers and the icon
components run hover animations; to fix, make the icons respect an external
control by adding a boolean prop (e.g., disableHover) to the Icon components and
pass disableHover={isAnimated} here when using the imperative start/stop API,
then update each icon implementation to skip its internal
onHoverStart/onHoverEnd logic when disableHover is true; alternatively (if you
prefer the icons to control hover), remove the onMouseEnter/onMouseLeave
handlers and imperative ref calls from this file and rely solely on the icon’s
internal hover handlers.
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: 4
♻️ Duplicate comments (1)
components/ui/dots-horizontal-icon.tsx (1)
1-1: The "use client" directive issue has been resolved.The previously flagged critical issue about the missing
"use client"directive has been addressed. The component now correctly includes the directive at the top of the file.
🧹 Nitpick comments (5)
components/ui/dots-horizontal-icon.tsx (2)
27-64: Consider removing the unusedasynckeyword.The
startcallback is markedasyncbut never usesawait. Since the parallel animations are intentional and don't need to be awaited, theasynckeyword is unnecessary.🔎 Optional refactor
- const start = useCallback(async () => { + const start = useCallback(() => { animate(
79-94: Consider adding accessibility attributes.The SVG element lacks accessibility attributes such as
aria-labelorrole. If the icon is interactive or conveys meaning, consider adding these attributes to improve screen reader support.🔎 Example enhancement
<motion.svg ref={scope} xmlns="http://www.w3.org/2000/svg" width={size} height={size} viewBox="0 0 24 24" fill="none" stroke={color} strokeWidth={strokeWidth} strokeLinecap="round" strokeLinejoin="round" className={`cursor-pointer ${className}`} onHoverStart={disableHover ? undefined : start} onHoverEnd={disableHover ? undefined : stop} + role="img" + aria-label="More options" >Note: Adjust the
aria-labelbased on the icon's semantic meaning in your application, or pass it as a prop if the meaning varies by context.lib/examples.ts (1)
48-51: Consider lazy-loading only the requested example.
getExampleBySlugcallsgetExamples(), which reads all example files from disk even when only one is needed. As the registry grows, this becomes increasingly inefficient.🔎 Proposed refactor for lazy loading
export function getExampleBySlug(slug: string) { - const examples = getExamples(); - return examples.find((example) => example.slug === slug); + const example = EXAMPLE_REGISTRY.find((ex) => ex.slug === slug); + if (!example) return undefined; + + let code = ""; + try { + code = fs.readFileSync( + path.join(process.cwd(), example.filePath), + "utf8", + ); + } catch (error) { + console.error( + `Error reading file for example ${example.slug} at ${example.filePath}:`, + error, + ); + code = "// Error loading component code"; + } + + return { + componentName: example.componentName, + slug: example.slug, + createdBy: example.createdBy, + description: example.description, + tags: example.tags, + code, + component: example.component, + }; }components/ui/bookmark-icon.tsx (1)
24-50: Consider wrappingstartandstopinuseCallbackfor consistency.Other icon components in this PR (e.g.,
mail-filled-icon.tsx,user-check-icon.tsx,users-icon.tsx) useuseCallbackto memoize animation functions. This ensures stable function references across renders.🔎 Proposed fix
+import { forwardRef, useImperativeHandle, useCallback } from "react"; -import { forwardRef, useImperativeHandle } from "react";- const start = async () => { + const start = useCallback(async () => { await animate( ".bookmark-body", { scaleY: 0.9, y: 2, }, { duration: 0.18, ease: "easeOut", }, ); - }; + }, [animate]); - const stop = async () => { + const stop = useCallback(async () => { await animate( ".bookmark-body", { scaleY: 1, y: 0, }, { duration: 0.18, ease: "easeInOut", }, ); - }; + }, [animate]);components/ui/magnifier-icon.tsx (1)
23-54: Consider memoizing animation functions and removing unnecessary wrappers.The
handleHoverStart/handleHoverEndwrappers add indirection without benefit. For consistency with other icons, consider usinguseCallbackand passingstart/stopdirectly.🔎 Proposed fix
+import { forwardRef, useImperativeHandle, useCallback } from "react"; -import { forwardRef, useImperativeHandle } from "react";- const start = async () => { + const start = useCallback(async () => { await animate( ".magnifier-group", { x: [0, 1, 0, -1, 0], y: [0, -1, -2, -1, 0], rotate: [0, -5, 5, -5, 0], }, { duration: 1, ease: "easeInOut" }, ); - }; + }, [animate]); - const stop = () => { + const stop = useCallback(() => { animate( ".magnifier-group", { x: 0, y: 0, rotate: 0 }, { duration: 0.2, ease: "easeOut" }, ); - }; + }, [animate]); useImperativeHandle(ref, () => ({ startAnimation: start, stopAnimation: stop, })); - const handleHoverStart = () => { - start(); - }; - - const handleHoverEnd = () => { - stop(); - }; - return ( <motion.svg ref={scope} - onHoverStart={disableHover ? undefined : handleHoverStart} - onHoverEnd={disableHover ? undefined : handleHoverEnd} + onHoverStart={disableHover ? undefined : start} + onHoverEnd={disableHover ? undefined : stop}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
components/examples/component-wrapper.tsxcomponents/examples/x-sidebar.tsxcomponents/navbar.tsxcomponents/ui/bookmark-icon.tsxcomponents/ui/dots-horizontal-icon.tsxcomponents/ui/filled-bell-icon.tsxcomponents/ui/home-icon.tsxcomponents/ui/magnifier-icon.tsxcomponents/ui/mail-filled-icon.tsxcomponents/ui/twitter-x-icon.tsxcomponents/ui/types.tscomponents/ui/unordered-list-icon.tsxcomponents/ui/user-check-icon.tsxcomponents/ui/users-icon.tsxlib/examples.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- components/navbar.tsx
- components/ui/types.ts
- components/ui/home-icon.tsx
- components/examples/x-sidebar.tsx
- components/ui/unordered-list-icon.tsx
- components/ui/filled-bell-icon.tsx
- components/ui/twitter-x-icon.tsx
- components/examples/component-wrapper.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
components/ui/dots-horizontal-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-11)
components/ui/mail-filled-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-11)
components/ui/user-check-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-11)
components/ui/users-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-11)
components/ui/bookmark-icon.tsx (1)
components/ui/types.ts (1)
AnimatedIconProps(1-11)
🪛 GitHub Actions: CI
lib/examples.ts
[warning] 1-1: Code style issues found in lib/examples.ts. Run Prettier with --write to fix.
🔇 Additional comments (12)
components/ui/dots-horizontal-icon.tsx (1)
6-120: Well-structured animated icon component.The implementation is clean and follows React best practices:
- Proper use of
forwardRefanduseImperativeHandlefor imperative animation control- Correct
useCallbackdependencies- Well-coordinated staggered animations with appropriate timing
- Transform origins properly set for center-based scaling
- Hover behavior correctly respects the
disableHoverprop- Clear TypeScript typing with exported handle interface
The component integrates well with the shared
AnimatedIconPropstype and provides a consistent API for the icon library.lib/examples.ts (3)
1-3: LGTM! Imports are appropriate.The Node.js fs/path modules confirm this is server-side code, which is correct for Next.js data fetching in Server Components or API routes.
5-18: LGTM! Registry structure is clear and extensible.The static registry with inline comments provides a straightforward way to add new examples.
20-46: Error handling successfully addresses previous review feedback.The try-catch block with fallback and contextual logging prevents server crashes when files are missing or paths are incorrect. This implementation resolves the concern raised in the previous review.
components/ui/mail-filled-icon.tsx (2)
23-54: LGTM!The animation logic is well-implemented with proper
useCallbackmemoization and correct dependencies. The rotateX animation creates a nice mail-opening effect.
56-81: LGTM!The SVG structure is clean and properly integrates with the animation system. Good use of the
perspective-distantclass for 3D transforms and conditional hover handlers.components/ui/bookmark-icon.tsx (1)
67-94: LGTM!The wrapper pattern using
motion.divfor hover handling while keeping the SVG separate is a valid approach. The component correctly implements the imperative handle and conditional hover behavior.components/ui/user-check-icon.tsx (2)
1-50: LGTM!Good implementation with proper
useCallbackmemoization. The parallel animations (avatar bounce and checkmark draw) create a nice coordinated effect.
83-123: LGTM!The SVG structure is well-organized with proper grouping for animations. The
transformOriginsettings ensure smooth scaling effects.components/ui/magnifier-icon.tsx (1)
56-89: LGTM!Good use of
overflow: visibleto prevent clipping during the wiggle animation. The SVG structure is well-organized.components/ui/users-icon.tsx (2)
23-80: LGTM!Animation logic is well-implemented with proper
useCallbackmemoization. The parallel animations for primary and secondary users create a nice layered interaction effect.
82-117: LGTM!The SVG structure is clean with well-organized groups for the two user elements. The component correctly implements the imperative handle and conditional hover behavior.
| import { forwardRef, useImperativeHandle } from "react"; | ||
| import { AnimatedIconProps } from "./types"; | ||
| import { motion, useAnimate } from "motion/react"; |
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.
Missing "use client" directive.
This component uses React hooks and motion event handlers, which require client-side rendering in Next.js.
🔎 Proposed fix
+"use client";
+
import { forwardRef, useImperativeHandle } from "react";
import { AnimatedIconProps } from "./types";
import { motion, useAnimate } from "motion/react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { forwardRef, useImperativeHandle } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; | |
| "use client"; | |
| import { forwardRef, useImperativeHandle } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; |
🤖 Prompt for AI Agents
In components/ui/magnifier-icon.tsx lines 1-3, the file is missing the "use
client" directive required for React hooks and motion event handlers to run
client-side in Next.js; add the exact line "use client" as the very first line
of the file (before any imports) so the component is treated as a client
component, then verify hooks (forwardRef, useImperativeHandle, useAnimate) and
motion usage remain unchanged.
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | ||
| import { AnimatedIconProps } from "./types"; | ||
| import { motion, useAnimate } from "motion/react"; |
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.
Missing "use client" directive.
This component uses React hooks and motion event handlers, which require client-side rendering in Next.js. Other icon components in this PR (e.g., bookmark-icon.tsx, user-check-icon.tsx) include the directive.
🔎 Proposed fix
+"use client";
+
import { forwardRef, useImperativeHandle, useCallback } from "react";
import { AnimatedIconProps } from "./types";
import { motion, useAnimate } from "motion/react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; | |
| "use client"; | |
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; |
🤖 Prompt for AI Agents
components/ui/mail-filled-icon.tsx around lines 1 to 3: this component uses
React hooks and motion handlers but is missing the required "use client"
directive for Next.js client-side rendering; add a top-level "use client"
directive as the very first line of the file (before any imports) so the
component is treated as a client component, then keep existing imports and
exports unchanged.
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | ||
| import { AnimatedIconProps } from "./types"; | ||
| import { motion, useAnimate } from "motion/react"; |
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.
Missing "use client" directive.
This component uses React hooks and motion event handlers, which require client-side rendering in Next.js.
🔎 Proposed fix
+"use client";
+
import { forwardRef, useImperativeHandle, useCallback } from "react";
import { AnimatedIconProps } from "./types";
import { motion, useAnimate } from "motion/react";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; | |
| "use client"; | |
| import { forwardRef, useImperativeHandle, useCallback } from "react"; | |
| import { AnimatedIconProps } from "./types"; | |
| import { motion, useAnimate } from "motion/react"; |
🤖 Prompt for AI Agents
In components/ui/users-icon.tsx around lines 1 to 3, the component uses React
hooks and motion handlers but is missing the Next.js "use client" directive; add
the line "use client" as the very first non-comment line of the file (before any
imports) so the component is rendered client-side, keeping the existing imports
and exports unchanged.
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 (2)
lib/examples.ts (2)
7-18: Consider adding type definitions for better type safety.Defining an interface for registry entries would improve type safety and provide better IDE support when adding new examples.
🔎 Proposed type definition
Add this interface before the registry:
interface ExampleRegistryEntry { componentName: string; slug: string; createdBy: string; filePath: string; component: React.ComponentType; description: string; tags: string[]; }Then type the registry:
-const EXAMPLE_REGISTRY = [ +const EXAMPLE_REGISTRY: ExampleRegistryEntry[] = [
48-51: Optional: Consider performance for large example sets.Currently,
getExampleBySlugreads all example files from disk viagetExamples()to find one. For a small number of examples this is fine, and Next.js caching likely mitigates the impact. If the registry grows significantly, you could optimize by reading only the needed file.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/examples.ts
🔇 Additional comments (1)
lib/examples.ts (1)
20-46: Error handling looks good.The try-catch block appropriately handles file read failures with logging and a fallback placeholder. This correctly addresses the previous review concern about unhandled exceptions.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
public/r/layout-sidebar-right-collapse-icon.json (1)
1-19: Registry content is out of sync with source files and requires updates.The embedded TypeScript code in the registry JSON does not match the actual source files. The
layout-sidebar-right-collapse-icon.tsxfile has formatting differences:
Line 8-12 parameter formatting: The actual source uses multiline formatting for the forwardRef parameters (opening parenthesis on separate lines), while the registry embeds it as a single line.
Lines 66-67 motion.path elements: The actual source renders these on single lines, while the registry embeds them across multiple lines with properties on separate lines.
The
types.tsfile content matches correctly. Update the embedded TSX content in the registry JSON to match the actual source file formatting exactly.
🧹 Nitpick comments (2)
icons/layout-sidebar-right-collapse-icon.tsx (2)
10-10: MissingdisableHoverprop handling.The
AnimatedIconPropstype includes adisableHoverprop, but it's not being destructured or used. This prevents consumers from disabling hover-triggered animations when needed.🔎 Proposed fix
-( - { size = 24, color = "currentColor", strokeWidth = 2, className = "" }, - ref, - ) => { +( + { size = 24, color = "currentColor", strokeWidth = 2, className = "", disableHover = false }, + ref, + ) => {Then update the hover handlers:
const handleHoverStart = () => { + if (disableHover) return; start(); }; const handleHoverEnd = () => { + if (disableHover) return; stop(); };
33-38: Consider adding a dependency array touseImperativeHandle.The hook is missing a dependency array, causing the handle object to be recreated on every render. While functionally correct, adding dependencies improves performance.
🔎 Proposed fix
useImperativeHandle(ref, () => { return { startAnimation: start, stopAnimation: stop, }; - }); + }, []);Alternatively, wrap
startandstopinuseCallbackand include them in the dependency array if their behavior may change based on props.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
icons/index.tsicons/layout-sidebar-right-collapse-icon.tsxpublic/r/layout-sidebar-right-collapse-icon.json
🔇 Additional comments (2)
icons/index.ts (1)
166-166: LGTM!The new icon import and ICON_LIST entry follow the established patterns. The import is correctly placed in alphabetical order, and the keywords are consistent with similar layout icons.
Also applies to: 1342-1354
icons/layout-sidebar-right-collapse-icon.tsx (1)
48-69: Well-structured animated SVG component.The motion.svg wrapper with scoped animation targets (
.sidebar,.chevron) and proper SVG attributes follows the established pattern for animated icons in the codebase.
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: 2
🧹 Nitpick comments (2)
icons/layout-sidebar-right-collapse-icon.tsx (2)
33-38: Consider memoizing animation functions with useCallback.The
startandstopfunctions are recreated on every render, which means the imperative handle is also recreated. While this works correctly, wrapping these functions inuseCallbackwould prevent unnecessary recreations and improve performance slightly.🔎 Proposed refactor with useCallback
+ const start = useCallback(async () => { - const start = async () => { animate( ".chevron", { x: [0, 2, 0], opacity: [1, 0.7, 1] }, { duration: 1.5 }, ); animate( ".sidebar", { x: [0, 2, 0] }, { duration: 2 }, ); - }; + }, [animate]); + const stop = useCallback(() => { - const stop = () => { animate(".chevron", { x: 0, opacity: 1 }, { duration: 0.2 }); animate(".sidebar", { x: 0 }, { duration: 0.2 }); - }; + }, [animate]);Don't forget to import
useCallback:-import { forwardRef, useImperativeHandle } from "react"; +import { forwardRef, useImperativeHandle, useCallback } from "react";
40-46: Simplify by passing animation functions directly to hover handlers.The
handleHoverStartandhandleHoverEndfunctions are thin wrappers that add no additional logic. You can passstartandstopdirectly to theonHoverStartandonHoverEndprops for cleaner code.🔎 Proposed simplification
- const handleHoverStart = () => { - start(); - }; - - const handleHoverEnd = () => { - stop(); - }; - return ( <motion.svg ref={scope} - onHoverStart={handleHoverStart} - onHoverEnd={handleHoverEnd} + onHoverStart={start} + onHoverEnd={stop} xmlns="http://www.w3.org/2000/svg"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
icons/layout-sidebar-right-collapse-icon.tsx
🧰 Additional context used
🪛 GitHub Actions: CI
icons/layout-sidebar-right-collapse-icon.tsx
[warning] 1-1: Prettier formatting issues found in this file. Run 'prettier --write' to fix.
🔇 Additional comments (1)
icons/layout-sidebar-right-collapse-icon.tsx (1)
1-3: Motion library API compatibility verified.Motion version 12.23.26 fully supports the
useAnimatehook andmotion.svgcomponent as used in this file. All imports resolve correctly to their sources (motion/reactand./types).
| const start = async () => { | ||
| animate( | ||
| ".chevron", | ||
| { x: [0, 2, 0], opacity: [1, 0.7, 1] }, | ||
| { duration: 1.5 }, | ||
| ); | ||
| animate( | ||
| ".sidebar", | ||
| { x: [0, 2, 0] }, | ||
| { duration: 2 }, | ||
| ); | ||
| }; |
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.
Remove async or add await for animation sequencing.
The start function is declared async but doesn't await any of the animate calls. This means:
- Both animations fire simultaneously (parallel execution)
- The
asynckeyword is unnecessary if parallel execution is intended - If sequential execution was intended, add
awaitbefore eachanimatecall
🔎 Proposed fix: Remove async if parallel execution is intended
- const start = async () => {
+ const start = () => {
animate(
".chevron",
{ x: [0, 2, 0], opacity: [1, 0.7, 1] },
{ duration: 1.5 },
);
animate(
".sidebar",
{ x: [0, 2, 0] },
{ duration: 2 },
);
};🔎 Alternative: Add await if sequential execution is intended
const start = async () => {
- animate(
+ await animate(
".chevron",
{ x: [0, 2, 0], opacity: [1, 0.7, 1] },
{ duration: 1.5 },
);
- animate(
+ await animate(
".sidebar",
{ x: [0, 2, 0] },
{ duration: 2 },
);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const start = async () => { | |
| animate( | |
| ".chevron", | |
| { x: [0, 2, 0], opacity: [1, 0.7, 1] }, | |
| { duration: 1.5 }, | |
| ); | |
| animate( | |
| ".sidebar", | |
| { x: [0, 2, 0] }, | |
| { duration: 2 }, | |
| ); | |
| }; | |
| const start = () => { | |
| animate( | |
| ".chevron", | |
| { x: [0, 2, 0], opacity: [1, 0.7, 1] }, | |
| { duration: 1.5 }, | |
| ); | |
| animate( | |
| ".sidebar", | |
| { x: [0, 2, 0] }, | |
| { duration: 2 }, | |
| ); | |
| }; |
| const start = async () => { | |
| animate( | |
| ".chevron", | |
| { x: [0, 2, 0], opacity: [1, 0.7, 1] }, | |
| { duration: 1.5 }, | |
| ); | |
| animate( | |
| ".sidebar", | |
| { x: [0, 2, 0] }, | |
| { duration: 2 }, | |
| ); | |
| }; | |
| const start = async () => { | |
| await animate( | |
| ".chevron", | |
| { x: [0, 2, 0], opacity: [1, 0.7, 1] }, | |
| { duration: 1.5 }, | |
| ); | |
| await animate( | |
| ".sidebar", | |
| { x: [0, 2, 0] }, | |
| { duration: 2 }, | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In icons/layout-sidebar-right-collapse-icon.tsx around lines 15 to 26, the start
function is marked async but does not await the animate calls so they run in
parallel and the async is unnecessary; either remove the async keyword to keep
parallel animations, or (if you want them to run sequentially) add await before
each animate call (await animate(...); await animate(...);) so the second
animation starts after the first completes.
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 (2)
icons/layout-sidebar-right-collapse-icon.tsx (2)
1-3: Fix Prettier formatting issues.The CI pipeline detected formatting issues in this file. Please run the formatter before merging.
#!/bin/bash # Run Prettier to fix formatting issues prettier --write icons/layout-sidebar-right-collapse-icon.tsx
15-22: Remove async keyword for parallel animations.The
startfunction is markedasyncbut doesn't await theanimatecalls, so they run in parallel and theasynckeyword is unnecessary. If parallel execution is intended (which appears to be the case given the different durations), remove theasynckeyword.🔎 Proposed fix
- const start = async () => { + const start = () => { animate( ".chevron", { x: [0, 2, 0], opacity: [1, 0.7, 1] }, { duration: 1.5 }, ); animate(".sidebar", { x: [0, 2, 0] }, { duration: 2 }); };
🧹 Nitpick comments (1)
icons/layout-sidebar-right-collapse-icon.tsx (1)
36-42: Consider passing animation functions directly to hover handlers.The
handleHoverStartandhandleHoverEndwrappers simply forward tostart()andstop(). You could pass these functions directly to the motion.svg handlers for slightly cleaner code.🔎 Optional simplification
- const handleHoverStart = () => { - start(); - }; - - const handleHoverEnd = () => { - stop(); - }; - return ( <motion.svg ref={scope} - onHoverStart={handleHoverStart} - onHoverEnd={handleHoverEnd} + onHoverStart={start} + onHoverEnd={stop}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
icons/layout-sidebar-right-collapse-icon.tsxpublic/r/layout-sidebar-right-collapse-icon.json
🔇 Additional comments (6)
public/r/layout-sidebar-right-collapse-icon.json (1)
1-20: LGTM! Registry configuration is well-structured.The JSON registry configuration correctly defines:
- Schema reference for validation
- Motion dependency for animations
- Both the icon component and shared types file
- Proper registry type annotations
icons/layout-sidebar-right-collapse-icon.tsx (5)
5-12: LGTM! forwardRef implementation is correct.The component properly uses forwardRef with typed parameters and sensible prop defaults. While React 19 allows ref as a regular prop, forwardRef remains fully supported and is appropriate here for the imperative handle pattern.
24-27: LGTM! Stop function correctly resets animations.The
stopfunction properly resets both animated elements to their initial states with a quick 0.2s duration, providing smooth animation cancellation.
29-34: LGTM! Imperative handle correctly exposes animation controls.The
useImperativeHandlehook properly exposes the animation API through the forwarded ref, allowing parent components to programmatically control animations.
44-65: LGTM! SVG structure and animation setup are correct.The motion.svg correctly:
- Uses
ref={scope}foruseAnimatetargeting- Attaches hover handlers for interactive animations
- Applies motion.path with classNames that match the animation selectors
- Properly spreads all icon props (size, color, strokeWidth, className)
69-71: LGTM! Proper component export with displayName.The displayName aids debugging in React DevTools, and the default export is appropriate for this icon component.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.