Order summary component challenge hub

Community feedback

• 

  T

  Alex•2,010

  @AlexKMarshallPosted almost 3 years ago

  This looks good to me on a big enough monitor. Semantics are pretty good. It needs a <main> element wrapping the content. And the links at the bottom of the card don't need headings inside them. Probably the <h2> for 'Annual Plan' is unnecessary too, as this is more of a list item, but that one is a bit more questionable.

  You don't need divs inside your links. Just style the <a> tag directly, that will make your markup more concise.

  Your design breaks a bit under screen-size of 385px. It starts to overflow horizontally which we want to avoid. This is because you've put an explicit with of 375px on your container. That should be avoided, should aim to handle at least down to 300px wide screen. Instead, don't have a width property at all. Just let the content be the width it needs to be naturally, and you can have a max-width to stop it getting to wide on bigger screens. You don't appear to have done it in this solution, but avoid using the height property for the same reason.

  Well done for using rem instead of px for font-sizes, that's very important.

  Avoid using margin where possible. Especially in a flex container you can just use gap instead. It means that the parent controls the spacing of its children, which is much easier to maintain.

  Marked as helpful

  0