Card Preview

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi @LucatoNine, well done on your second challenge! Those are some good questions you have, and I want to quickly comment as well!

  About the structure, I agree with what @correlucas said, especially about the text content. You only really need one div for all the text, and for things where more alignment is needed (like the prices for instance), then I would use another div. Generally, if no alignment or positioning is needed, you don't need to use a div. In terms of semantic HTML, however, you can use more semantic tags, espeically main, which is a necessary landmark for every site. That's also the reason for one of your accessibility issues right now in the report. All you need to do is change one of the divs (perhaps the one at the topmost level) to a main tag, and that should help get rid of the issue.

  One other comment: I'm looking at your site on my laptop, and the desktop view is huge! It's occupying almost my whole screen. Also, the height of the image container doesn't seem to be the same as the text one, so try adding height: 100% to the image container. I'd also remove the huge margin-top you have for the card preview, and perhaps try making some of the margins/padding a bit smaller, as I'm not sure why everything looks so big on my screen (which already is at 80% of the actual size).

  Anyway, keep going, you're doing great, and I hope we're able to help you out!

  1
• 

  Lucas 👾•104,340

  @correlucasPosted about 2 years ago

  👾Hello @LucatoNine, Congratulations on completing this challenge!

  ❓ Answering your question:

  1.You've used a lot of div, its not a problem since the solution is working, but you can clean the structure removing many of these divs, for example, in the content column, where there is all text, you need to use div only for the button and the pricing section, the rest of the elements can stand alone.

  2.Something you can do to improve the image that needs to change between mobile and desktop is to use <picture> instead of <img> wrapped in a div. You can manage both images inside the <picture> tag and use the html to code to set when the images should change setting the device max-width depending of the device (phone / computer) Here’s a guide about how to use picture: https://www.w3schools.com/tags/tag_picture.asp

  ✌️ I hope this helps you and happy coding!

  1