@LeonardoRFragosoSubmitted almost 2 years ago
Chris Jay
@chrisjay358All comments
- @chrisjay358Posted almost 2 years ago
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
- @LeonardoRFragosoSubmitted almost 2 years ago@chrisjay358Posted almost 2 years ago
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
- @aimal-qaziSubmitted almost 2 years ago
Finally done it
@chrisjay358Posted almost 2 years agoHi 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 helpful1 - @alexpenadevSubmitted almost 2 years ago
Let me know if anything can be improved. I am just returning to frontend mentor after a long break.
@chrisjay358Posted almost 2 years agoHi 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 helpful1 - The color of the
- @MelvinAguilarSubmitted about 2 years ago
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. 😊✌️
@chrisjay358Posted almost 2 years agoHi 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 - @rafaeldgeoSubmitted almost 2 years agoWhat 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
@chrisjay358Posted almost 2 years agoHi 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 helpful0
