Skip to content
Submitted over 2 years ago

Order Summary Component

Elodie•50
@elodiehg
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Order summary component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

I'm not sure about how to display the hero's image into my div card. Any feedback is welcome !

Community feedback

  • riverCodes•300
    @riverCodesPosted over 2 years ago

    Hi, good job so far on this challenge, the layout of your elements within the card looks great.

    However, there are a few things that can be improved. I'm mostly going to comment on what you should do to fix the issues detected in your HTML and Accessibility, as per the website.

    • Try not to use id selectors to style elements, use class selectors instead. ID's are more often used to target elements in JavaScript, not CSS.
    • You should always wrap the main content of your page in a <main> element. It is rendered the same way as a <div> but adds semantic meaning to your HTML, and helps screen readers access the main page of your content easily. In your case, this would be the <div id="card">
    • Also wrap the 'attributions' part of your content within a <footer> tag. Again, this helps screen-readers access it easily.
    • Always add alt attributes to your images. When the image is purely decorative, use an alt tag but keep it blank (alt=" "). Otherwise, alt tags should describe the image and are used by visually-impaired users.
    • You are using back-slashes "" in your image locators, switch those to forward-slashes "/" instead.
    • You can simply use background-image property on your <body> element. You don't need a div just for that.

    There are a few more problems as well. Your main content is not centered to your page, and your design is not responsive. It looks too small on large screens and also starts to break on smaller screens.

    I recently finished this challenge as well and I faced the same issues as well. This comment is already too long so I won't talk about that now. I can make a separate comment on how to tackle those, let me know if you would want that.

    Marked as helpful
    1
  • kirty•380
    @kirtymeenaPosted over 2 years ago

    Hi @ElodieHalgand,

    Use semantic HTML and use alt in the image tag. In img tag use "/" rather than backslash. doing so will remove the HTML issues and Accessibility Issues. something like this : <img src="images/illustration-hero.svg" id="hero" alt="hero" >

    Overall it looks good.

    keep Coding 👍

    Marked as helpful
    0
  • Ciceron•940
    @MarcusTuliusCiceronPosted over 2 years ago

    Hi, congrats on completing this challenge,

    I usually choose to display image as a background image in an empty div on which i will force the dimension. If needed I can change background image with media querries to have mobile image or desktop image when needed. Other solution is to use image tag but i find it less conveniant to make it responsive as it will not resize automatically according box size.

    Hope this will help

    Have a good day :D

    Marked as helpful
    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord