Sandra - Portfolio #43
Conversation
… button component
… with styled component
… in the contact-component
…l for projectcard and blogpostcard
Appilistus
left a comment
Appilistus
left a comment
There was a problem hiding this comment.
You have done such a great job as always!! The site looks good on every device, and you got high scores on both lighthouse and WAVE! The code and component structure is clean and easy to follow, also the names of components are easy to understand.
I added a few comments on your code, hope it helps ;)
Well done!👏🏻
There was a problem hiding this comment.
You look very confident and professional! Love the picture🤩🙌🏻
There was a problem hiding this comment.
Clean and well structured App.jsx! I learned from your code that having
element is good practice for SEO, accessibility etc. 💫 Well done!| @@ -0,0 +1,44 @@ | |||
| import styled from "styled-components" | |||
|
|
|||
| export const StyledButton = styled.button` | |||
There was a problem hiding this comment.
Do you use outside of this component? Maybe no need to export if it's local? :)
Also, I would recommend to style instead of . It's better for accessibility💡 I write more about this in the projectCard.jsx file!
| <h3>{title}</h3> | ||
| <p>{description}</p> | ||
| {link ? ( | ||
| <a href={link} target="_blank" rel="noopener noreferrer"> |
src/components/Skills/Skills.jsx
Outdated
| background-color: ${({ theme }) => theme.colors.primary}; | ||
| color: ${({ theme }) => theme.colors.secondary}; | ||
| padding: 0 0 5rem; | ||
| width: 100%; |
src/components/Tech/Tech.jsx
Outdated
| background-color: ${({ theme }) => theme.colors.primary}; | ||
| color: ${({ theme }) => theme.colors.secondary}; | ||
| padding: 0 0 5rem; | ||
| width: 100%; |
There was a problem hiding this comment.
I see you are using {theme} everywhere - that's great! I should also start using it...
src/index.css
Outdated
There was a problem hiding this comment.
Maybe you can remove everything if you don't use them?😄
| <p className="date">{date}</p> | ||
| <h3>{title}</h3> | ||
| <p>{description}</p> | ||
| {link ? ( |
There was a problem hiding this comment.
You are already doing this link check within the Button component :)
| {link ? ( | ||
| <Button as="a" href={link} target="_blank" rel="noopener noreferrer" icon="/link.svg">Read article</Button> | ||
| ) : ( | ||
| <Button icon="/link.svg">Read article</Button> |
There was a problem hiding this comment.
What does this do when there is no link?
There was a problem hiding this comment.
At the moment, the button has no action. I kept it in the UI to follow the Figma design even when no link is available. And I wrote it like this since I was planning to add articles later on, and then the button will behave as intended.
| <p>Lorem ipsum dolor sit amet consectetur adipisicing elit. Voluptatem, laborum! Maxime animi nostrum facilis distinctio neque labore consectetur beatae eum ipsum excepturi voluptatum, dicta repellendus incidunt fugiat, consequatur rem aperiam.</p> | ||
| <ThemeProvider theme={theme}> | ||
| <GlobalStyle /> | ||
| <a href="#main" className="skip-link">Skip to main content</a> |
There was a problem hiding this comment.
Nice that you include a "skip to content" link!
|
|
||
| <Content $position={position}> | ||
| <Tags> | ||
| {tags && tags.map((tag, i) => ( |
There was a problem hiding this comment.
A tip! You can make use of optional chaining here instead of checking if the tags are defined first:
tags?.map(...)
Npahlfer
left a comment
Npahlfer
left a comment
There was a problem hiding this comment.
Solid use of theme and very easy to follow and you are even using exports in index files! Really nice work!
3 participants
🚀 Netlify
🔗 https://sandrahagevall.netlify.app/
🎨 Figma design
🔗 https://www.figma.com/design/kOo9DGnKtIEUW7WRAvHPQC/Portfolio-design---Sandra?node-id=1-1856&t=bWeypHNcHdW4WWu2-0