Design comparison
Solution retrospective
Any feedback is welcome. 😃
Community feedback
- @ApplePieGiraffePosted over 3 years ago
Greetings, SnowFox! 👋
Congratulations on completing your first Frontend Mentor challenge! 🎉 Nice work on this one! 🙌 The different image that you added is a nice touch! 😀
One super tiny suggestion I have is to add a max-width to the card so that it isn't so wide when the layout first changes from desktop to mobile.
And I don't think you should use an
<article>
tag for the container holding the image in the card component (since it isn't really a self-contained element that makes sense by itself). An<article>
tag for the entire card and<div>
s for the containers inside it might be a little better for semantics. Check out this thorough article from CSS Tricks to learn more about the<article>
and<section>
tags and their usage. 😉Keep coding (and happy coding, too)! 😁
1@ISnowFoxIPosted over 3 years ago@ApplePieGiraffe Hi there and thank you.
I agree with you on that one, I'll set it to max-width:375px; so it stays the same for mobile view.
I tried to make the HTML more semantic but seems like I ended up using the wrong ones haha.
I will read the article you linked and also all make the changes as you suggested 😃.
1 - @palgrammingPosted over 3 years ago
Well first I got to say that I do like the image change you made 🌟🌟🌟🌟🌟. It helps the overall design and feel of the project. Maybe if you made the photo grayscale and tried to match the purple overlay more it would be even better
The only other thing I see is your attribution if changing location in the mobile view and hiding behind your layout
1@ISnowFoxIPosted over 3 years agoThank you, I will make the photo grayscale, as for the attribution, I forgot to check it on my phone 😁. Will fix both of them soon 😃.
0 - @ISnowFoxIPosted over 3 years ago
I updated the code as you said, made the image grayscale, and fixed the HTML. I couldn't center the card component and have the attribution at the bottom of the page, so I added an empty header to body in order to have space between the two using flex. So having an empty header is kinda bugging me 😅.
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord