Mikaelas js-project-portfolio #54
Conversation
|
Im missing the "Please include a link to your Figma design and a Netlify link." at PR description ;) |
daniellauding
left a comment
daniellauding
left a comment
There was a problem hiding this comment.
Great work! Clean and fresh code.
| @@ -1 +1,3 @@ | |||
| # Portfolio | |||
|
|
|||
| mikaela-js-project-portfolio.netlify.app | |||
There was a problem hiding this comment.
I didnt see any images, maybe intentionally? on the projects articles etc
| <style> | ||
| @import url('https://fonts.googleapis.com/css2?family=Hind:wght@300;400;500;600;700&display=swap'); | ||
| @import url('https://fonts.googleapis.com/css2?family=Montserrat:ital,wght@0,100..900;1,100..900&display=swap'); | ||
| </style> |
There was a problem hiding this comment.
Nice that you added it in index.html, it loads before react mounts and causes fewer layout shifts, me usually add it as:
/* index.css */
@import url
but import in CSS is slower (blocks rendering) and can cause FOUT/FOIT (flash of unstyled text).
👍
| @@ -0,0 +1,31 @@ | |||
| import styled from "styled-components"; | |||
|
|
|||
| export const StyledSection = styled.div` | |||
There was a problem hiding this comment.
StyledSection is well-structured and the variant mapping to theme tokens is a solid pattern. A couple of small suggestions below
| /* Tablet and up --> */ | ||
| @media (min-width: ${({ theme }) => theme.breakpoints.tablet}) { | ||
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
display: flex and flex-direction don’t need to be repeated inside media queries.
| @@ -0,0 +1,31 @@ | |||
| import styled from "styled-components"; | |||
|
|
|||
| export const StyledSection = styled.div` | |||
There was a problem hiding this comment.
You may consider switching to styled.section for semantic benefits
| @@ -0,0 +1,24 @@ | |||
| import styled from "styled-components" | |||
| import { CardImage, Tag, Button, SvgIcon } from "../ui/ui" | |||
There was a problem hiding this comment.
Nice that you built some core component =)
| <CardImage src={article.image} alt={article.alt}/> | ||
| <Tag tag={article.date} /> | ||
| <CardTitle title={article.name} /> | ||
| <BodyText text={article.description} /> |
There was a problem hiding this comment.
Nice! Would maybe recommend trying to use <Title/Text> as component and variant can be maybe "Card" or something :] but nice
| <Tag tag={article.date} /> | ||
| <CardTitle title={article.name} /> | ||
| <BodyText text={article.description} /> | ||
| <Button href={article.link} text= "Read article" variant="article"> |
There was a problem hiding this comment.
text= " a nice little spacing maybe accidentally came here
| export const ArticleSection = ({ variant }) => { | ||
| return ( | ||
| <StyledSection variant={variant} gap="56px" padding="120px 16px"> | ||
| <SectionTitle title="My words" variant={variant} textAlign="left"/> |
| <Avatar /> | ||
| <StyledDiv> | ||
| <BodyText text={aboutMe.name} weight="semibold" fontSizeT="tablet" | ||
| fontSizeD="desktop"fontFam="heading"/> |
There was a problem hiding this comment.
looks a bit wierd with fontSizeD="desktop"fontFam="
no spacing? :D
3 participants
Please include a link to your Figma design and a Netlify link.