Nathan Andrade• 40
@Nathan4Andrade
Submitted
Franco Juarez
@Juarrison
All comments
- Franco Juarez• 220
@Juarrison
Posted
Hi @Nathan4Andrade. Congratulations on your project. I think you could try some changes to improve the code. First of all, you could explore the semantic tags that HTML has and decrease the use of divs. Here is an interesting link about it: https://www.w3schools.com/html/html5_semantic_elements.asp
On the other hand, the mobile version is not responsive. I suggest that in the mobile version you incorporate the flex-direction: column property along with some changes to the size of the image. Greetings, I hope this comment has been useful to you.
0 - Marius Bareanu• 60
@MariusSB-code
Submitted
Any feedback is appreciated!
Franco Juarez• 220@Juarrison
Posted
Hi @MariusSB-code. Congratulations for your work. First of all, I think you could explore some semantic html tags. Here is some relevant information about it: https://www.w3schools.com/html/html5_semantic_elements.asp
Secondly, I think that instead of creating a special container for the image and applying the background-image property, it is more convenient to work with the image as an element itself and discard the container. On the other hand, you could use the flex-direction: column property for the mobile version and, with the application of media queries, change this property to flex-direction: row for the tablet and desktop versions. Greetings, I hope this comment has been useful.
Marked as helpful
0 - Patryk Besler• 355
@beslerpatryk
Submitted
Franco Juarez• 220@Juarrison
Posted
Hi @beslerpatryk. Congratulations for your work. I find very interesting the use of semantic tags and the incorporation of media queries to make the project responsive. I make a small observation. I noticed that in mobile resolutions the fixed property you used for the footer makes it overlap with the <main>. I think you could remove that property to avoid such a conflict. Greetings, I hope this brief comment will be useful for the project. Good start to the week.
Marked as helpful
1 - Nico• 10
@nrougier
Submitted
Franco Juarez• 220@Juarrison
Posted
Hi @nrougier. Congratulations on the work you did on this project. I think it would be interesting if you try some code changes. First of all, the name of the main HTML file should be index, because this way, you won't have problems with the server. On the other hand, I invite you to explore some semantic tags to moderate the use of divs. Here is some useful information about it: https://www.w3schools.com/html/html5_semantic_elements.asp
Finally, you could try the following buy button:
<button> <img src="images/icon-cart.svg" alt="shopping cart icon"> Add to Cart</button>
Greetings, I hope this comment is useful to you. Cheers!
Marked as helpful
1 - Mukesh Patidar• 20
@patidarmk
Submitted
Franco Juarez• 220@Juarrison
Posted
Hi @patidarmk. Good work! I suggest you try a few small changes. First, the background color proposed by the challenge is hsl(212, 45%, 89%). On the other hand, the card title font should have another font ('Outfit', sans-serif;). Finally, I think you could explore some semantic HTML tags like <article>, <footer>, among others. Here is some interesting documentation about it: https://www.w3schools.com/html/html5_semantic_elements.asp Greetings. I hope all this will be useful to you on this path of web development.
0 - umar• 20
@Umar29
Submitted
Franco Juarez• 220@Juarrison
Posted
Hi @Umar29. I congratulate you for the work you did in this challenge. I think you could try a few simple changes to improve your project. First of all, you could change the color of the main paragraph and the strikethrough price. If I remember correctly, the challenge proposed the following color: hsl(228, 12%, 48%). On the other hand, you organized the price of the card using a table (<table>). Perhaps you could use a <div> with the flex property. I hope this comment will be useful in your project. Greetings and have a good weekend.
Marked as helpful
0 - Jasham• 80
@jasham
Submitted
Franco Juarez• 220@Juarrison
Posted
Hi @jasham. I wanted to congratulate you for the work you did on this challenge. Here's a little suggestion: use more semantic HTML tags. This would give better accessibility to the page. In this particular case, you could use tags like <article>, <footer>, <header>, <h1,h2,h3...>, among others. I hope this comment is useful to you. Greetings and congratulations for the challenge! Looks great.
Marked as helpful
0 - John A.• 10
@johnnyakin
Submitted
Quite easy but I mixed CSS & Bootstrap to center the element.
Franco Juarez• 220@Juarrison
Posted
Hi @johnnyakin. Congratulations for your work. I notice a small detail in the solution you presented. The credits and authorship of the code should be on the outside of the card. On the other hand, I think it would be interesting for you to explore the semantic html tags. I hope this little observation will be useful to you. Greetings and good week.
Marked as helpful
0 - DInosMpo• 210
@DinosMpo
Submitted
Please tell me the best solution or any mistake that i made. Thank you
Franco Juarez• 220@Juarrison
Posted
Hello, DinosMpo. Congratulations for your work. From what I can see, the HTML tag tag is missing the alt text. This text is a clear description of the image that helps improve the accessibility of the web page. Here is some useful information so you can improve this aspect of the code: https://www.w3schools.com/tags/att_img_alt.asp I hope this has been helpful. Greetings.
Marked as helpful
0 - Sumit• 90
@SumitMish23
Submitted
Hi Everyone ! This is my first project after years and I have just started with the frontend.Please check and add your review comments. Regards !
Franco Juarez• 220@Juarrison
Posted
Hi @SumitMish23. I found the way you did this challenge very interesting. As a suggestion it seems to me that you could explore more the semantic tags in the HTML. Greetings and good week.
0
