HTML and CSS solution with Flexbox

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted almost 4 years ago

  Hi Maya,

  Overall this looks really good 👍

  I'd recommend you just take another look at the semantics of your html.

  1. You've tried to use headings in order, which is great, but try to really think about what makes sense as a heading.. . Eg what would you expect content to be under a heading of "803k"? Would that make sense on a contents page? Probably not. So it shouldn't be a heading. I think you only need one heading max on this, and the stats should be combined into a single meaningful tag like a paragraph or list item (using spans inside to style the number and word respectively)

  2. At a push this could be an article. But remove the header main and footer from it. Header and footer are allowed technically inside an article, but they actually make for a really poor screenreader experience. Assistive tech users would find it much easier to navigate when those are only used for main page/site content, like one header per page just like you have one h1 per page .

  Have a look at my solution if youd like to see how I approached it.

  Hope all that is helpful

  2

  ---

  

  Maya Buser De•40

  @mayabuserdePosted almost 4 years ago

  @grace-snow thank you for your feedback and for sharing your solution. this was very helpful!

  0
• 

  Renszo Camacho•1,615

  @RenszCamachoPosted almost 4 years ago

  Hello. Nice job!. You got a few issues,

  • One of them is the main tag, which must be involving your article tag.

  • It's a 'Good Practice' use a class instead of an Html tag. Like <header class="header">. There's something called 'specificity'. You could have more than one footer and don't want the same style.

  • Image tag should have an alternative text <img src="images/image-victor.jpg" alt="user image - victor" />

  • About the images in the background, you can play with the viewport width and viewport height. Something like this: background-position: right 52vw bottom 38vh, left 49vw top 51vh;

  Hopefully, it helps.

  2

  ---

  

  T

  Grace•31,270

  @grace-snowPosted almost 4 years ago

  @RenszCamacho it is completely valid to leave alt attribute intentionally empty if you want assistive tech to skip over it.

  In this case though, I agree it is a meaningful image that should have a description.

  But never use words or phrases like "image of" in alt text as the element is already announced as an image

  2

  

  Renszo Camacho•1,615

  @RenszCamachoPosted almost 4 years ago

  @grace-snow Make sense. Thanks for share you knowledge 🙇‍♂️

  0

  

  Maya Buser De•40

  @mayabuserdePosted almost 4 years ago

  @RenszCamacho Thank you for your feedback. Quick question... if I use vw and vh for the placement of the background images, will they stay in place on different screen sizes? Thanks!

  0

  

  Renszo Camacho•1,615

  @RenszCamachoPosted almost 4 years ago

  @mayabuserde I am a rookie, so I don't know if my answer is the most accurate. 😊Maybe at some point, you will need to use media queries. But overall it is. It's quite adaptive for most devices.

  I've been checking your code.

  • I would do. background-position: right 50vw bottom 50vh, left 50vw top 50vh; and remove the media queries.

  I hope I've helped. 😊

  1
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted almost 4 years ago

  Greetings, Maya Buser De! 👋

  I just wanted to say, congratulations on completing your first Frontend Mentor challenge! 🎉 Good work on this one! 👍

  Simply follow the helpful suggestions given above, and I think you'll be good to go! 😀

  Keep coding (and happy coding, too)! 😁

  0

  ---

  

  Maya Buser De•40

  @mayabuserdePosted almost 4 years ago

  @ApplePieGiraffe Thank you and happy coding to you too :)

  0