@naomi-pham
@itush
Submitted
Hello Developers👋
Another nice project to practice DOM manipulation with toggle visual state and I enjoyed building it 😄
As always, I'd really appreciate if you could answer the following:👇
1.What did I do wrong? 2.What did I do right? 3.How can I improve? common mistakes?
🙏Thanks in advance Happy Coding😄
@naomi-pham
Posted
Hi Tusher, great job with using Tailwind and vanilla JavaScript for your project. I think your Tailwind code is very concise and easy to read. I also learned about the Tailwind's utility space-between space-y-2 from your code. It is amazing!!
However, I noticed that you put some parts like <section>Hello, thanks for setting up the Chess Club ...</section> in a separate paragraph; as a result, it does not appear in the same block of the parent. You can resolve this by wrapping it in the same div of the content besides the profile pic. This is also metioned by Igo in the 3rd point of his comment.
Keep up the amazing work!
@sarah27h
Submitted
Hi alI, I have some questions about best practices. 1- For the icon alt attribute, I have added it like that but for the other image I have left it blank
<img src="./images/icon-star.svg" alt="rating star" class="card__icon">
<img src="./images/illustration-thank-you.svg" alt="" class="card__img">
Should I have used SVG instead of img tag?
2- I have added an event listener on ul and another one for submit button Is it better to use only one on the out container and then check the target one?
<section class="card__container js-card__container">
<img src="./images/icon-star.svg" alt="rating star" class="card__icon">
<h1 class="card__heading">
How did we do?
</h1>
<p class="card__text">
Please let us know how we did with your support request. All feedback is appreciated to help us improve our offering!
</p>
<ul class="card__rating js-card__rating">
<li>1</li>
<li>2</li>
<li>3</li>
<li>4</li>
<li>5</li>
</ul>
<button class="card__btn js-card__btn">Submit</button>
</section>
@naomi-pham
Posted
Hi, your site looks great! Love that you added some transition to the rating circle.
About your question on the image tag, I think it's better to add the alt tag so that visitors can know what your image is about when it fails to load.
I also have a small recommendation regarding the position of your card. You can add display: flex and min-height: 100vh to the body to put the card in center of the page:
body {
display:flex;
justify-content: center;
align-items: center;
min-height: 100vh;
Happy coding! 😊
Marked as helpful
@jutin0852
Submitted
@naomi-pham
Posted
Hi, great job on the responsive design! However, I noticed that your component <div id="read"> has a margin-left of 10px which makes it unaligned with the title on the mobile screen. I think you can fix it by placing your h1 and <div id="read"> inside a flexbox with a gap value:
<div class="feature">
<h1></h1>
<div id="read"></div>
</div>
.feature {
display: flex;
flex-direction: column;
gap: 0.5rem;
}
On mobile screen, you can add flex-direction: row so it would align horizontally.
Marked as helpful
@kramlig22
Submitted
@naomi-pham
Posted
Hi, I think for such a small project, your HTML tags are perfectly fine. I can easily locate sections in your code.
If you have trouble naming sections, it might be helpful to take a look at Bootstrap card component. Like in this example, you can start with a high-level card, then card-header, card-body, card-footer.
<div class="card"">
<div class="card-header">
<img />
</div>
<div class="card-body">
<h6 class="card-subtitle">Card subtitle</h6>
<h5 class="card-title"></h5>
<p class="card-text"></p>
</div>
<div class="card-footer">
</div>
</div>
Hope it helps 😊
Marked as helpful