feat/ addded passport, airplane, travel-bag, hotel icons#54
feat/ addded passport, airplane, travel-bag, hotel icons#54Abhijit-Jha merged 4 commits intoitshover:masterfrom
Conversation
|
@Aryan-205 is attempting to deploy a commit to the itshover's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds four new animated React SVG icons (AirplaneIcon, HotelIcon, PassportIcon, TravelBag) with shared animation types, exports, and registry entries; each exposes imperative Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom Pre-merge Checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @icons/airplane-icon.tsx:
- Around line 1-89: Delete the large 89-line commented-out implementation block
in icons/airplane-icon.tsx (the commented imports, forwardRef component
definition for AirplaneIcon, its methods start/stop, useImperativeHandle, the
motion.svg JSX, AirplaneIcon.displayName and the default export); remove the
entire commented region so the file contains only active code (or an intended
minimal stub) and rely on VCS history if the old implementation must be
recovered.
In @icons/index.ts:
- Around line 187-190: The import list mixes a component named HotelIcon with a
source file "./hostel-icon", causing a naming mismatch; update either the file
or the component so names align: rename the file "./hostel-icon" to
"./hotel-icon" or rename the exported component inside that file to HostelIcon
and update the import to match, and mirror the same naming convention referenced
in lib/icons.ts so all imports/exports (e.g., PassportIcon, AirplaneIcon,
TravelBag, HotelIcon/HostelIcon) are consistently named.
- Around line 1225-1229: The keywords array for the icon object with name
"passport-icon" contains a duplicate entry; update the object (the entry where
name === "passport-icon" and icon === PassportIcon) to remove the repeated
"passport" so the keywords array only includes unique terms (e.g.,
["passport","travel","transportation"]).
In @lib/icons.ts:
- Around line 362-365: The icon entry with name "hotel icon" currently has a
mismatched path "/icons/hostel-icon"; update the object so the display name and
path are consistent—either change the name to "hostel icon" or update the path
to "/icons/hotel-icon" in the icons array entry (the object containing name:
"hotel icon" and path: "/icons/hostel-icon") to resolve the naming
inconsistency.
In @public/r/airplane-icon.json:
- Around line 9-10: Remove the large commented-out legacy implementation at the
top of the file (the block of commented imports, the old forwardRef AirplaneIcon
definition, its start/stop/useImperativeHandle logic, and its commented export)
so only the active AirplaneIcon implementation remains; ensure the real imports,
the currently defined functions start and stop, the useImperativeHandle call,
AirplaneIcon.displayName and the default export are preserved and run a quick
formatter to clean spacing.
🧹 Nitpick comments (4)
icons/hostel-icon.tsx (2)
28-31: Add dependency array touseImperativeHandle.The
useImperativeHandlehook is missing its dependency array. Without it, the ref object will be recreated on every render, which is inefficient and inconsistent with the pattern used inpassport-icon.tsx.🔎 Proposed fix
- useImperativeHandle(ref, () => ({ - startAnimation: start, - stopAnimation: stop, - })); + useImperativeHandle(ref, () => ({ + startAnimation: start, + stopAnimation: stop, + }), [start, stop]);
69-69: Remove commented-out debug code.This appears to be leftover debug/development code that should be cleaned up before merging.
🔎 Proposed fix
export default HotelIcon; - -// stroke="#ff34f8"icons/travel-bag.tsx (1)
36-39: Add dependency array touseImperativeHandle.Similar to
hostel-icon.tsx, theuseImperativeHandlehook is missing its dependency array. This should include[start, stop]for consistency and to avoid unnecessary recreations of the ref object.🔎 Proposed fix
- useImperativeHandle(ref, () => ({ - startAnimation: start, - stopAnimation: stop, - })); + useImperativeHandle(ref, () => ({ + startAnimation: start, + stopAnimation: stop, + }), [start, stop]);public/r/hostel-icon.json (1)
10-10: Remove commented debug code.The embedded component code contains a leftover debug comment at the end:
// stroke=\"#ff34f8\". This should be removed before merging.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
icons/airplane-icon.tsxicons/hostel-icon.tsxicons/index.tsicons/passport-icon.tsxicons/travel-bag.tsxlib/icons.tspublic/r/airplane-icon.jsonpublic/r/hostel-icon.jsonpublic/r/layout-sidebar-right-collapse-icon.jsonpublic/r/passport-icon.jsonpublic/r/registry.jsonpublic/r/travel-bag.jsonregistry.json
🔇 Additional comments (12)
public/r/layout-sidebar-right-collapse-icon.json (1)
1-20: LGTM!The registry manifest and embedded icon component code follow the established pattern correctly. The animation logic using
useAnimatefor chevron and sidebar elements is properly implemented.registry.json (1)
24-40: LGTM!The new registry entries for
airplane-icon,hostel-icon,passport-icon, andtravel-bagare correctly structured and consistent with existing icon entries. Since this is an auto-generated file, ensurenpm run registry:buildis run to keep it in sync with source files.Also applies to: 1724-1740, 2132-2148, 2829-2845
icons/passport-icon.tsx (2)
1-50: LGTM!The component correctly implements the
forwardRefpattern withuseImperativeHandle. TheuseCallbackhooks properly includeanimatein the dependency array, anduseImperativeHandlecorrectly depends on[start, stop].
52-90: Verify the globe animation reset behavior on repeated hovers.The
stop()function sets.globepaths to{ pathLength: 1 }, butstart()animates from{ pathLength: [0, 1] }. On the first hover, this works correctly. However, on subsequent hovers, sincepathLengthis already1, the[0, 1]keyframe animation should still replay from 0. If the visual effect doesn't reset as expected during testing, consider settingpathLength: 0instop()to ensure the animation visibly replays.public/r/registry.json (1)
24-40: LGTM!The public registry entries mirror the root
registry.jsoncorrectly. All four new icon entries are properly structured.Also applies to: 1724-1740, 2132-2148, 2829-2845
icons/airplane-icon.tsx (1)
90-184: LGTM!The active implementation is well-structured with proper use of
useCallback,useImperativeHandlewith correct dependencies, and smooth animation sequences for the airplane and wind streaks.icons/travel-bag.tsx (1)
41-88: LGTM!The SVG structure and animation logic for the travel bag open/close effect are well-implemented. The coordinated skew and translate transforms create a nice visual effect.
lib/icons.ts (1)
6-9: New icon entries look good.The airplane, passport, and travel bag entries follow the existing naming conventions and structure.
Also applies to: 426-429, 570-573
public/r/hostel-icon.json (1)
1-20: Registry structure looks good.The hostel-icon registry entry follows the established pattern with proper dependencies, file references, and component implementation.
public/r/passport-icon.json (1)
1-20: LGTM!The passport-icon registry entry is well-structured. The component properly uses
useImperativeHandlewith the correct dependency array[start, stop], and the animation sequencing with async/await is clean.public/r/travel-bag.json (1)
1-20: LGTM!The travel-bag registry entry follows the established pattern. The multi-element animation (front-flap, elements, back-flap) creates a nice opening effect.
Note: For consistency with passport-icon, you could add
[start, stop]to theuseImperativeHandledependency array, though it's not strictly necessary since the callbacks are memoized.icons/index.ts (1)
211-215: New ICON_LIST entries look good.The airplane-icon, hotel-icon, and travel-bag entries are properly structured with appropriate keywords and follow the existing patterns.
Also applies to: 784-788, 970-974
| import PassportIcon from "./passport-icon"; | ||
| import AirplaneIcon from "./airplane-icon"; | ||
| import TravelBag from "./travel-bag"; | ||
| import HotelIcon from "./hostel-icon"; |
There was a problem hiding this comment.
Import naming inconsistency.
HotelIcon is imported from "./hostel-icon". This ties to the broader naming inconsistency flagged in lib/icons.ts. Consider aligning the file name with the component name (either rename the file to hotel-icon.tsx or rename the component to HostelIcon).
🤖 Prompt for AI Agents
In @icons/index.ts around lines 187 - 190, The import list mixes a component
named HotelIcon with a source file "./hostel-icon", causing a naming mismatch;
update either the file or the component so names align: rename the file
"./hostel-icon" to "./hotel-icon" or rename the exported component inside that
file to HostelIcon and update the import to match, and mirror the same naming
convention referenced in lib/icons.ts so all imports/exports (e.g.,
PassportIcon, AirplaneIcon, TravelBag, HotelIcon/HostelIcon) are consistently
named.
| { | ||
| name: "hotel icon", | ||
| path: "/icons/hostel-icon", | ||
| }, |
There was a problem hiding this comment.
Naming inconsistency: "hotel icon" vs "hostel-icon" path.
The display name is "hotel icon" but the path references "hostel-icon". This mismatch could confuse users and should be aligned. Consider renaming either the display name to "hostel icon" or updating the path to "/icons/hotel-icon" for consistency.
🤖 Prompt for AI Agents
In @lib/icons.ts around lines 362 - 365, The icon entry with name "hotel icon"
currently has a mismatched path "/icons/hostel-icon"; update the object so the
display name and path are consistent—either change the name to "hostel icon" or
update the path to "/icons/hotel-icon" in the icons array entry (the object
containing name: "hotel icon" and path: "/icons/hostel-icon") to resolve the
naming inconsistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @icons/airplane-icon.tsx:
- Around line 43-47: The stop callback resets the ".wind1" element to x:12,y:-4
which mismatches the initial ".wind1" state (x:10,y:-6) and causes a visual
jump; update the stop callback inside the stop function to animate ".wind1" back
to the same initial coordinates (x:10, y:-6) used when the component
mounts/initializes so start/stop positions match, ensuring the animate calls for
".wind1" and any related initial values in the component are consistent with
each other.
🧹 Nitpick comments (1)
icons/travel-bag.tsx (1)
12-34: Minor: Inconsistent async declaration betweenstartandstop.
startis declared asasync(line 12) butstopis not (line 26). Neither function actually awaits anything, so theasynckeyword onstartis unnecessary. Consider removing it for consistency.🔎 Proposed fix
- const start = useCallback(async () => { + const start = useCallback(() => { animate(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
icons/airplane-icon.tsxicons/hostel-icon.tsxicons/index.tsicons/travel-bag.tsxpublic/r/airplane-icon.jsonpublic/r/hostel-icon.jsonpublic/r/passport-icon.jsonpublic/r/travel-bag.json
🚧 Files skipped from review as they are similar to previous changes (2)
- public/r/passport-icon.json
- public/r/hostel-icon.json
🔇 Additional comments (6)
icons/airplane-icon.tsx (1)
1-95: Clean implementation following established patterns.The component correctly uses
forwardRef,useImperativeHandle, anduseAnimatefor animation control. The hover-triggered animations and imperative API are well-structured.icons/travel-bag.tsx (1)
1-92: LGTM!The TravelBag component follows the established icon patterns with proper animation controls for the bag opening/closing effect. The skew-based animation creates a nice visual effect.
icons/index.ts (1)
211-215: New icon entries look good.The new icon entries for airplane-icon, hotel-icon, travel-bag, and passport-icon are properly structured with relevant keywords. The duplicate "passport" keyword issue from the past review appears to be resolved.
Also applies to: 784-788, 970-974, 1225-1229
public/r/travel-bag.json (1)
1-20: LGTM!The registry manifest correctly declares the motion dependency and includes the component source with proper type definitions.
public/r/airplane-icon.json (2)
9-10: Embedded code contains same wind1 position inconsistency.The embedded component code has the same animation state mismatch for
wind1as noted in the source file review. When the source file is fixed, ensure this registry manifest is regenerated.
1-20: Registry entry structure is correct.The manifest properly declares dependencies and includes the component with type definitions.
feat/ addded passport, airplane, travel-bag, hotel icons, all tests successful, inform me if the animation requires changes
Screen.Recording.2026-01-07.at.10.32.31.AM.mov
Screen.Recording.2026-01-07.at.10.33.18.AM.mov
Screen.Recording.2026-01-07.at.10.33.59.AM.mov
Screen.Recording.2026-01-07.at.10.34.33.AM.mov
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.