@LeonardoRFragoso
Submitted
Chris Jay
@chrisjay358
All comments
- Chris Jay• 220
@chrisjay358
Posted
Hi Leonardo Rodrigues Fragoso, this is a fine piece of work here, nothing to fix. Great design I wonder how you were able to get an accurate design (space and sizing). Thumbs up, I'm happy for you.
Here are the only accessibility issues you can change up.
- You can change your
<div class="container">to<article class="container">and wrap it in a<main>for semantic reasons.
Once again, Leonardo Rodrigues Fragoso, I think you did amazing work, a great one even, and would like you to share how you got accurate sizing and spacing. Thanks
0 - You can change your
@LeonardoRFragoso
Submitted
Chris Jay• 220@chrisjay358
Posted
Bom dia, Leonardo Rodrigues Fragoso, você tem um trabalho incrível e pode até ficar ótimo, melhor ainda.
- Você deve alterar a
<seção>para<artigo>é ótimo para um conteúdo independente. - Você também deve adicionar algum preenchimento à sua área de texto para dar um pouco de beleza.
- Você também pode trabalhar na cor da fonte.
0- Você deve alterar a
- Qazi Aimal• 150
@aimal-qazi
Submitted
Finally done it
Chris Jay• 220@chrisjay358
Posted
Hi Qazi Aimal, you did fine work, congrats on that.
- My contribution is that you should add a margin-bottom to the container class in order to give a visual hierarchy to the attribution class.
- You can also use an
<article> instead of a <div>for semantics
Above that, it is great work you go there, keep doing you.
Marked as helpful
1 - Alexander Pena• 30
@alexpenadev
Submitted
Let me know if anything can be improved. I am just returning to frontend mentor after a long break.
Chris Jay• 220@chrisjay358
Posted
Hi Alexander, I think you did a good job, and if you want to make it even better, here are my observations that you can add.
- The color of the
<p>text.
.qr-body p { color: var(--gray-blue); }- You could also have used
<article class ="container"></article> instead of <div class ="container">It is better for semantics and helps screen readers also.
- You can reduce the padding on the inside also.
Above all this I called out, you did a pretty amazing job considering you've been off for a while. Keep winning and growing Alexander Pena!
Marked as helpful
1 - The color of the
- Melvin Aguilar 🧑🏻💻• 61,440
@MelvinAguilar
Submitted
Hi there 👋, I’m Melvin, and this is my solution for this challenge. 🚀
🛠️ Built With:
- SASS. 🎨
- Flexbox layout. 📏
Any suggestions on how I can enhance this solution or achieve even better performance are welcome!
Thank you. 😊✌️
Chris Jay• 220@chrisjay358
Posted
Hi Melvin, that's great work you've done here. It is the striking resemblance for me. I just went through your code and wonder why you don't use this.
* { padding: 0; margin: 0; }I think it saves you from long codes.
0 - Rafael Dias• 720
@rafaeldgeo
Submitted
What are you most proud of, and what would you do differently next time?
I'm proud I have gotten to use media queries to create the two layouts.
What challenges did you encounter, and how did you overcome them?
It's complicate define the different images for mobile and desktop. I managed to solve this problem using tag .
What specific areas of your project would you like help with?
I would like to help with the use tag
Chris Jay• 220@chrisjay358
Posted
Hi Rafael, I think you did an amazing job! I like it and if you would like to improve it, here are a few tips I think you should include.
- Give more room for visual hierarchy (space between blocks)
main { min-height: calc(100vh - 30px); margin-bottom: 3rem; }. That way on a mobile view the footer doesn't rub into the card itself. You could also have a margin-bottom for hierarchy also between the <main> and the <footer>.- This is all for now. Hope this meets you well.
Marked as helpful
0
