Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Responsive landing page using CSS flex box

@owocoded

Desktop design screenshot for the Order summary component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


just submitting a frontend challenge. any feedback on how to improve will be welcome

Community feedback

@danielemanca1983

Posted

Hi Yusuf, Here is some feedback:

  1. Your solution's mark-up code could greatly improve by using semantic mark-up tags, as someone else already mentioned, you could wrap the content in a <main> tag. It is preferable to use semantic html tags instead of aria roles.

  2. I would have enclosed the actual component inside a <article> tag instead, by perhaps assigning a class to it such as card and then all the inner children elements such as the image and text could use BEM methodology for their class names. You can take a look at what BEM is on this link about [BEM] (https://getbem.com/introduction)

  3. The image could be enclosed inside a <picture> tag for semantics.

  4. Typography: the font-family in your solution does not seem to match the ones in the original design.

  5. For the button you have actually used a <p> tag, you must used either an anchor or a <button> for that purpose as the element on the design is a call to action. It also does not have a drop-shadow as per the design.

  6. Cancel the order should also be using an anchor tag instead, as it also is a call to action. In general we would use a <p> tag only for regular text elements.

  7. I would suggest the use of classes instead of IDs, unless we need an ID to bind the html element to a piece of javascript code in order to add some interactive functionality to it.

It may seem like a lot, but believe me, as I am a front-end developer, I would get this sort of feedback whenever demoing a completed project to the visual designers, if there were mismatching elements.

Marked as helpful

0

@owocoded

Posted

@danielemanca1983 thank you very much, will surely work on it

1

@WhaleWellWell

Posted

Hi! Overall great project and code, but here is some advice I think you will find helpful:

  • I see that there is a div with a class of main: <div class="main">. To make this site better for screen readers you could add the role of main to your div or wrap your div in a <main> tag.
<main>
  <div class="main">
    CODE
  </div>
</main>

or:

<div role="main" class="main">
  CODE
</div>

It is suggested you use the <main> tag but either one will work. MDN docs explain it better than me :)

  • I see you have an <h4> in your order component for your smaller heading. I recommend you just use an <h2> instead because the heading tags are mainly for your site's outline and semantic order and not the for the style. If you want your text to look about the size of an <h4> that is fine, you can make it look like that in your CSS. If you want took look farther into this here is a the MDN docs on the heading tags. It has the explanation under "Usage Notes"

Hope you find my advice useful, WhaleWellWell

Marked as helpful

0

Please log in to post a comment

Log in with GitHub
Discord logo

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