Develop #11
Develop #11
Conversation
…' and change 'icon' to 'image' for skills
…ndle SVG elements conditionally
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors AboutTimeline and ExperienceCard for stronger typing and flexible icon handling by introducing new interfaces, updating data structures, and adjusting rendering logic.
- Introduced
SkillsType,WorkExperience, andCompaniesPropsinterfaces for type-safe data modeling. - Updated the
experiencesarray intimeline.tsxto uselogo/imagefields with the new interfaces. - Enhanced
ExperienceCardto render both SVG-based and image-based skill icons dynamically.
Reviewed Changes
Copilot reviewed 3 out of 8 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/lib/interfaces/skills.ts | Added SkillsType, WorkExperience, and CompaniesProps types. |
| src/components/ui/experience-card.tsx | Switched props to WorkExperience, and improved icon rendering. |
| src/app/(web)/(home)/sections/timeline.tsx | Migrated companyLogo/icon to logo/image and integrated SVG. |
Comments suppressed due to low confidence (1)
src/lib/interfaces/skills.ts:13
- [nitpick] The interface name
CompaniesPropssuggests multiple companies but defines a single company. Consider renaming it toCompanyPropsfor clarity.
export interface CompaniesProps {
| @@ -0,0 +1,18 @@ | |||
| export interface SkillsType { | |||
| title: string; | |||
| icon?: React.SVGProps<SVGSVGElement>; | |||
There was a problem hiding this comment.
The icon property is typed as React.SVGProps<SVGSVGElement> (an SVG props interface) but the code treats it as a React element. Consider changing this to ReactElement<SVGSVGElement> or JSX.Element to match usage.
| icon?: React.SVGProps<SVGSVGElement>; | |
| icon?: JSX.Element; |
| <div className="hidden absolute top-3 bottom-0 right-full mr-7 md:mr-[3.25rem] w-px bg-foreground/20 sm:block"> </div> | ||
| <div className="flex flex-col gap-12"> | ||
| {experiences.map(({ works, title, companyLogo }, index) => ( | ||
| {experiences.map(({ works, title, logo }, index) => ( |
There was a problem hiding this comment.
Using the array index as the React key can lead to rendering issues. Consider using a stable identifier such as the company title or a unique ID instead.
1 participant
This pull request refactors the
AboutTimelineandExperienceCardcomponents to improve type safety and enhance flexibility in handling skill icons and images. It introduces new TypeScript interfaces, updates data structures, and modifies rendering logic to accommodate both image-based and SVG-based icons.TypeScript Interface Enhancements:
SkillsType,WorkExperience, andCompaniesPropsinsrc/lib/interfaces/skills.tsto define structured types for skills, work experiences, and companies. This improves type safety across components.Data Structure Updates:
experiencesarray insrc/app/(web)/(home)/sections/timeline.tsxto use the newCompaniesPropsinterface, replacingcompanyLogowithlogoandiconwithimagein skill definitions.ExperienceCardcomponent insrc/components/ui/experience-card.tsxto use theWorkExperienceinterface, replacing the previous custom props structure.Rendering Logic Improvements:
ExperienceCardto differentiate between SVG-based icons and image-based icons, allowing dynamic handling of both types.AboutTimelinecomponent to use the newlogoproperty instead ofcompanyLogofor company logos.