refactor(about): move to custom page #436
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/src/app/[locale]/about/page.tsxOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@nerdfish/config' imported from /eslint.config.js WalkthroughThe pull request introduces significant changes to the website's structure, focusing on the "About" page implementation. The existing localized "About" page has been removed and replaced with a new component that provides a more comprehensive and structured approach to displaying personal and professional information. New components for various sections, such as skills, hero, FAQ, and split layouts, have been created. Additionally, internationalization (i18n) dictionaries have been updated with detailed content for both English and Dutch locales. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
apps/web/src/app/components/keyword-list.tsx (2)
10-25: Consider removing redundant aria-labelThe
aria-labelis currently set to the same value as the visible text content, which is redundant and may cause screen readers to announce the text twice.- aria-label={children}
29-37: Make Marquee animation parameters configurableThe component has hardcoded values for
pauseOnHover,duration, andrepeatwhich might not suit all use cases. Consider making these configurable through props while keeping the current values as defaults.- <Marquee pauseOnHover duration={20000} repeat={5} {...props}> + <Marquee + pauseOnHover + duration={props.duration ?? 20000} + repeat={props.repeat ?? 5} + {...props} + >apps/web/src/app/components/faq.tsx (1)
33-35: Consider extracting prose classes to a constantThe component uses multiple prose-related classes that could be extracted to a constant for better maintainability.
const PROSE_CLASSES = 'prose prose-xl pt-md text-primary text-xl' // Usage <AccordionContent className={PROSE_CLASSES}>apps/web/src/app/components/skills.tsx (2)
2-17: Consider lazy loading icons to improve initial bundle sizeThe component imports all icons upfront, which could impact the initial bundle size. Consider lazy loading icons using dynamic imports.
const iconMap = { javascript: () => import('@repo/design-system/lib/icons').then(m => m.JavascriptIcon), // ... other icons } // Usage with React.lazy and Suspense const IconComponent = React.lazy(async () => { const IconModule = await iconMap[skill]() return { default: IconModule } })
21-36: Add type safety for skill mappingThe skill mapping could be more type-safe by ensuring all skills from the schema have corresponding icons.
type Skill = typeof skills[number]; // Ensure type safety by checking if all skills have icons type SkillIconMapType = Record<Skill, React.ElementType>; const hasAllSkills = (map: Partial<SkillIconMapType>): map is SkillIconMapType => { return skills.every(skill => skill in map); }; const skillIconMap = { javascript: JavascriptIcon, // ... other mappings }; if (!hasAllSkills(skillIconMap)) { throw new Error('Missing icon mappings for some skills'); }apps/web/src/app/[locale]/about/page.tsx (1)
93-107: Consider dynamic skill items loadingInstead of hardcoding skill items, consider loading them from a configuration file or CMS for easier maintenance.
- <Skills> - <SkillItem skill="javascript" /> - <SkillItem skill="react" /> - <SkillItem skill="node" /> - <SkillItem skill="tailwind" /> - <SkillItem skill="next" /> - <SkillItem skill="sanity" /> - <SkillItem skill="webflow" /> - <SkillItem skill="typescript" /> - <SkillItem skill="vscode" /> - <SkillItem skill="css" /> - <SkillItem skill="html" /> - <SkillItem skill="git" /> - <SkillItem skill="figma" /> - </Skills> + <Skills> + {skills.map(skill => ( + <SkillItem key={skill} skill={skill} /> + ))} + </Skills>packages/i18n/dictionaries/en.json (1)
137-137: Fix extra spaces in CTA subtitleThere are unnecessary spaces in the CTA subtitle text.
- "subtitle": "Custom websites and applications tailored to help your business grow and stand out.", + "subtitle": "Custom websites and applications tailored to help your business grow and stand out.",packages/i18n/dictionaries/nl.json (2)
136-138: Remove extra spaces in subtitle text.There are unnecessary spaces after "bedrijf te laten groeien en" which should be removed for consistency.
- "subtitle": "Aangepaste websites en applicaties op maat om uw bedrijf te laten groeien en op te vallen.", + "subtitle": "Aangepaste websites en applicaties op maat om uw bedrijf te laten groeien en op te vallen.",
146-146: Remove extra spaces in subtitle text.There are unnecessary spaces after "plugins, features, animaties".
- "subtitle": "HTML, CSS, JS, het bouwen van kleine, middelgrote en grote webapplicaties met JavaScript libraries, frameworks, aangepaste plugins, features, animaties en het coderen van interactieve layouts. Ik heb ook full-stack developer ervaring en bovenal heb ik een grote interesse in leren en ben ik niet bang om nieuwe talen en uitdagingen aan te gaan.", + "subtitle": "HTML, CSS, JS, het bouwen van kleine, middelgrote en grote webapplicaties met JavaScript libraries, frameworks, aangepaste plugins, features, animaties en het coderen van interactieve layouts. Ik heb ook full-stack developer ervaring en bovenal heb ik een grote interesse in leren en ben ik niet bang om nieuwe talen en uitdagingen aan te gaan.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/web/src/app/[locale]/(tina)/about/page.tsx(0 hunks)apps/web/src/app/[locale]/about/page.tsx(1 hunks)apps/web/src/app/[locale]/page.tsx(2 hunks)apps/web/src/app/components/faq.tsx(1 hunks)apps/web/src/app/components/hero.tsx(2 hunks)apps/web/src/app/components/keyword-list.tsx(1 hunks)apps/web/src/app/components/skills.tsx(1 hunks)apps/web/src/app/components/split.tsx(1 hunks)packages/i18n/dictionaries/en.json(1 hunks)packages/i18n/dictionaries/nl.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/app/[locale]/(tina)/about/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Lint
- GitHub Check: E2E
- GitHub Check: TypeCheck
🔇 Additional comments (7)
apps/web/src/app/components/hero.tsx (1)
81-94: Well-structured component implementation!The
HeroCTAcomponent effectively combinesMagnetButtonandLinkwith proper type inheritance and consistent styling. The addition ofArrowRightIconenhances the visual feedback.apps/web/src/app/[locale]/about/page.tsx (1)
31-56: Comprehensive metadata implementation!The metadata generation is well-structured with proper handling of localization, SEO fields, and canonical/alternate URLs.
apps/web/src/app/[locale]/page.tsx (1)
75-75: Clean refactor using the new HeroCTA component!The replacement of the custom button implementation with
HeroCTAimproves consistency and reduces code duplication.packages/i18n/dictionaries/en.json (1)
126-169: Well-structured and comprehensive translations!The about page translations are thorough and well-organized, covering all necessary sections with detailed content.
packages/i18n/dictionaries/nl.json (3)
127-130: LGTM! Well-structured SEO metadata.The metadata section provides comprehensive SEO-friendly content with clear value propositions and proper keyword usage.
149-168: LGTM! Well-balanced personal and professional content.The content effectively combines personal interests with professional expertise, and includes appropriate alt text for images.
126-169: Verify translation consistency with English version.Let's ensure all keys in the about section are properly translated from English.
✅ Verification successful
Dutch translations are properly implemented ✅
All keys are properly structured and content is correctly translated to Dutch, with appropriate preservation of technical terms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare structure of English and Dutch about page translations # and check for potential English text in Dutch translations # First, extract and format the about section from both files echo "Extracting and comparing translation structures..." # Extract about section keys from both files rg -A 50 '"about": \{' packages/i18n/dictionaries/en.json > en_about.txt rg -A 50 '"about": \{' packages/i18n/dictionaries/nl.json > nl_about.txt # Compare structure (keys should match) diff <(grep -o '"[^"]*":' en_about.txt | sort) <(grep -o '"[^"]*":' nl_about.txt | sort) # Check for common English words in Dutch translations echo "Checking for potential untranslated content..." rg -i '\b(the|and|or|with|in|on|at|to|for|of|by)\b' packages/i18n/dictionaries/nl.jsonLength of output: 4610
Script:
#!/bin/bash # Description: Compare structure of English and Dutch translations with improved handling # Extract and compare about section keys echo "Comparing translation structures..." jq -r '.about | .. | select(objects) | keys[]' packages/i18n/dictionaries/en.json | sort > en_keys.txt jq -r '.about | .. | select(objects) | keys[]' packages/i18n/dictionaries/nl.json | sort > nl_keys.txt diff en_keys.txt nl_keys.txt # Show any keys present in English but missing in Dutch echo -e "\nKeys in English missing from Dutch:" comm -23 en_keys.txt nl_keys.txt # Show any extra keys in Dutch not in English echo -e "\nExtra keys in Dutch not in English:" comm -13 en_keys.txt nl_keys.txtLength of output: 729
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/design-system/components/arrow-link.tsx (2)
72-75: Consider improving ref type safety.The implementation is good, but there's room for improvement in type safety.
Consider using
forwardRefand proper generic types to avoid theas anytype assertion:import { forwardRef } from 'react' const ArrowLink = forwardRef<HTMLElement, ArrowLinkProps>(({ children, direction = 'right', className, href, as, ...props }, ref) => { const Tag = as ? motion(as) : href ? motion(Link) : motion.span const [, state] = useElementState() return ( <Tag {...props} href={href ?? '#'} className={cx( 'space-x-md text-primary inline-flex cursor-pointer items-center text-left text-lg font-bold !no-underline transition focus:outline-none', className, )} ref={ref} animate={state} > {/* ... rest of the component ... */} </Tag> ) }) ArrowLink.displayName = 'ArrowLink'
110-111: Maintain consistency in ref handling.Similar to ArrowLink, BackLink would benefit from improved ref handling.
Consider applying the same
forwardRefpattern to BackLink for consistency:const BackLink = forwardRef<HTMLElement, { href: string } & Pick<ArrowLinkProps, 'className' | 'children' | 'as'>>(({ href, className, children, as, ...props }, ref) => { const [, state] = useElementState() const Tag = as ? motion(as) : href ? motion(Link) : motion.span return ( <Tag {...props} href={href} className={cx( 'text-primary flex space-x-4 focus:outline-none', className, )} ref={ref} animate={state} > {/* ... rest of the component ... */} </Tag> ) }) BackLink.displayName = 'BackLink'Also applies to: 114-114
apps/web/src/app/[locale]/about/page.tsx (3)
74-86: Consider making image paths configurable.The image path
/uploads/about/daren-mountains.JPGis hardcoded. Consider moving it to the translations or a configuration file for better maintainability.- src="/uploads/about/daren-mountains.JPG" + src={t('professional.image.src')}
87-109: Consider making skills data-driven.The skills list is currently hardcoded. Consider moving it to the translations or a separate configuration file for better maintainability and easier updates.
Example approach:
const skills = t('my-toolbox.skills', { returnObjects: true }) as string[]; return ( <Skills> {skills.map(skill => ( <SkillItem key={skill} skill={skill} /> ))} </Skills> );
137-143: Consider making testimonial filters configurable.The testimonial filter is hardcoded to
['colleague', 'client']. Consider moving this to the translations or a configuration file for better maintainability.- filter={{ - type: ['colleague', 'client'], - }} + filter={{ + type: t('testimonials.filter.types', { returnObjects: true }), + }}packages/i18n/dictionaries/en.json (1)
138-138: Fix extra whitespace in CTA subtitle.The CTA subtitle contains extra whitespace characters that should be removed.
- "subtitle": "Custom websites and applications tailored to help your business grow and stand out.", + "subtitle": "Custom websites and applications tailored to help your business grow and stand out.",packages/i18n/dictionaries/nl.json (1)
138-138: Fix extra whitespace in CTA subtitle.The CTA subtitle contains extra whitespace characters that should be removed.
- "subtitle": "Aangepaste websites en applicaties op maat om uw bedrijf te laten groeien en op te vallen.", + "subtitle": "Aangepaste websites en applicaties op maat om uw bedrijf te laten groeien en op te vallen.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/app/[locale]/about/page.tsx(1 hunks)packages/design-system/components/arrow-link.tsx(2 hunks)packages/design-system/components/section.tsx(2 hunks)packages/i18n/dictionaries/en.json(1 hunks)packages/i18n/dictionaries/nl.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: TypeCheck
- GitHub Check: Lint
- GitHub Check: E2E
🔇 Additional comments (8)
packages/design-system/components/section.tsx (2)
77-77: LGTM! Good use of polymorphic component pattern.The addition of
as?: React.ElementTypeto the cta object follows React's best practices for polymorphic components.
99-101: LGTM! Proper prop forwarding.Clean implementation of forwarding the
asprop to ArrowLink component.packages/design-system/components/arrow-link.tsx (1)
64-64: LGTM! Type definition follows React conventions.The
as?: React.ElementTypetype is correctly defined for polymorphic component support.apps/web/src/app/[locale]/about/page.tsx (5)
1-57: LGTM! Well-structured metadata implementation.The metadata generation follows Next.js best practices with proper handling of:
- Internationalized titles and descriptions
- SEO metadata with OpenGraph image
- Canonical URLs and language alternates
59-73: LGTM! Clean Hero section implementation.The Hero section is well-structured with proper translation handling and clear call-to-action.
110-122: LGTM! Clean implementation of the Personal section.The section follows the same consistent structure as the Professional section.
123-129: LGTM! Well-structured CTA implementation.The CTA section is cleanly implemented with proper translation handling.
130-136: Consider validating the blog count parameter.The
BlogOverviewcomponent receives a hardcoded count of 2. Consider validating this parameter to ensure it's within acceptable bounds.
Summary by CodeRabbit
New Features
Documentation
Refactor