final portfolio site #46
Conversation
irisdgz
left a comment
irisdgz
left a comment
There was a problem hiding this comment.
Super solid work Mohammed! Im able to follow your code easily. Everything is organized, and divided into clear sections and it makes the whole project feel clean. The use of styled components, motion, and a theme gives it pro feel, and details like hover effects, accessibility and animations make a big difference. No majors fix than moving som animation props into variants or reduce the repetition. Seriously, great work!
| import styled from "styled-components"; | ||
| import { motion } from "framer-motion"; | ||
| import ArticleCard from "../components/ArticleCard"; | ||
| import articlesData from "../data/articles.json"; |
There was a problem hiding this comment.
Curious what you think about this—should we pass the articles data in as a prop from the parent component instead of importing the JSON directly here? Maybe it might make this component more reusable later if we want to show a different list of articles using the same layout.
| </Heading> | ||
|
|
||
| <ArticlesGrid> | ||
| {articles.map((article, index) => ( |
There was a problem hiding this comment.
Just a safety thought: if articles ever comes back undefined (like if the data fetch fails), this map might crash the page.
There was a problem hiding this comment.
Really solid work on this component! The structure is clean and super that you used aria-label on the section for accessibility.
| whileHover={{ scale: 1.05 }} | ||
| > | ||
| <img | ||
| src="/images/hero-main.png" |
There was a problem hiding this comment.
/images/... works locally, but if we deploy this to GitHub Pages (which usually lives at /repo-name/), this link might break because it looks for the image at the root of the server.
| @@ -0,0 +1,268 @@ | |||
| import styled from "styled-components"; | |||
| import { motion, useMotionValue, useSpring, useTransform } from "framer-motion"; | |||
|
|
|||
There was a problem hiding this comment.
useMotionValue, useSpring, useTransform these are new to me :)
| whileInView={{ opacity: 1 }} | ||
| transition={{ duration: 0.6, delay: 0.3 }} | ||
| viewport={{ once: true }} | ||
| > |
There was a problem hiding this comment.
Again it could be a variant if you want to reuse this pattern, but this is really clear and easy to follow thou.
There was a problem hiding this comment.
Your global styles look really clean and thoughtful. Good mix of everything. Super solid!
| @@ -1,12 +1,9 @@ | |||
| import { StrictMode } from 'react' | |||
| import { createRoot } from 'react-dom/client' | |||
| import { StrictMode } from "react"; | |||
| import { StrictMode } from 'react' | ||
| import { createRoot } from 'react-dom/client' | ||
| import { StrictMode } from "react"; | ||
| import { createRoot } from "react-dom/client"; |
There was a problem hiding this comment.
Nice structure, it’s clear how the page is built from the section imports
| <GlobalStyles /> | ||
|
|
||
| {/* Skip to main content - Accessibility */} | ||
| <a href="#main-content" className="skip-link"> |
There was a problem hiding this comment.
Nice that you use the skip to main content link!
| }; | ||
|
|
||
| // Icon | ||
| const LinkIcon = () => ( |
There was a problem hiding this comment.
Nice that you use SVGs for icons! It would be nice to have these icons in a icon directory, that way it's easy to find and update them when needed :)
| import styled from "styled-components"; | ||
|
|
||
| const Button = ({ children, href, icon, variant = "primary", ariaLabel }) => { | ||
| const ButtonComponent = variant === "primary" ? PrimaryButton : SecondaryButton; |
There was a problem hiding this comment.
I like that you have defined a "base button" and that you use it with the different versions of the button.
Any reason you didnt defined the variants styling in the same styled component with a $varant prop? :)
There was a problem hiding this comment.
I have no good reason, using a $variant prop with conditional styling in a single component would have been cleaner and more idiomatic. I'll refactor it!
There was a problem hiding this comment.
I have no good reason, using a $variant prop with conditional styling in a single component would have been cleaner and more idiomatic. I'll refactor it!
| import { motion, useMotionValue, useSpring, useTransform } from "framer-motion"; | ||
|
|
||
| // 3D Tilt Card Component - from old portfolio | ||
| const TiltCard = ({ children, $initialRotate = 0, style, ...props }) => { |
There was a problem hiding this comment.
Using the dollar sign for normal props can be confusing when dealing with styled components, as they usually use it for their props. Otherwise good work in this component!
|
Extensive use of theming and variants along with framer motion and clean components, really nice. I would have wanted to see some of the data in its own place instead of in the components and the icons in a separated place as well, but overall great job! :) |
3 participants
Figma design
Live Demo qabalany.com