-
Notifications
You must be signed in to change notification settings - Fork 50
Mimmi Portfolio #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Mimmi Portfolio #13
Conversation
… and buttons and adding a skip to main content link
|
Hi @mimmi-eriksson ! I have now reviewed your code, and compared your finished project to the Figma sketch. |
HIPPIEKICK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A11y
100% in Lighthouse ⭐
Regarding the requirement: Your portfolio should follow the Figma design
I think you’ve done a nice take on the design and you managed to implement it really well 🥳
Codewise
You have a very good structure of your project and you’ve used components and props in a good way. Your whole project is overviewable and clean. You’ve kept your entry point (App.jsx) clean which is good practice 👍 Nice to see that you’re utilising all of styled components possibilites, with global style and theming ⭐
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's hard to give a review to you code because everythig seems to be working very well! :D And i also like the design, straight on point, clear and easy to navigate! – here i just thought maybe on mobile mostly but also on other devices format it could be a lower line height? feels quite wide to me personally, could be slightly more near.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the body text i mean
alex91-html
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hej hej I have checked out your portfolio project, the code and structure with the components is very clear and makes it easy to undertand/navigate. I like the design as well, its also very clear and easy to navigate.
only small detail i could think about is the line spacing in the body text of the introduction, maybe could be less space between the lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the body text i mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so cool with media.js to set break points for different devices, i did the same! A mind labyrinth!
Netlify link: https://mimmi-eriksson.netlify.app/
Figma: https://www.figma.com/design/DWntRTXDyX18yQFDO9PHTL/Mimmi---Figma-designs-for-students?node-id=1078-906