I did it with flexbox

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey awesome work on this. Layout in general looks great but the component is not responsive.

  Some suggestions would be:

  • First, the font-style is different, you might want to check that one out.
  • Avoid using height: 100vh as this limits the element's height based on the screen. On your body tag replace it with min-height: 100vh this takes full height but lets the element expand if needed.
  • Always have a main element to wrap the main content of the page. On this, the .card should be using a main instead of div.
  • On the .card instead of width: 430px use max-width: 430px. Using the former makes the element have "fixed" width which makes it not-responsive to screen-changes unlike the latter.
  • Vector image should be hidden so use alt="" and add aria-hidden="true" attribute on the img. If an image acts only as decoration, always hide them at all times. Meaningful images needs to use meaningful alt value.
  • Also when using alt, avoid adding words that relates to "graphic" such as "illustration" since img is already an image, no need to describe it as one.
  • Always have an h1 on a page. On this one, since you use h2 on the order summary you would need to make the h1 screen-reader only text. Search up for sr-only on the net and see how it is used. The h1 will need that and the h1 will be placed as the first text-content element of the main. The text of the h1 will describe what is the main content is all about.
  • Music icon should be hidden by the method I mentioned above.
  • When using heading tag, make sure you aren't skipping a level. When you use h4 make sure that h1, h2, h3 are all present before it.

  Aside from those, great work again on this one.

  0

  ---

  

  samandavari•20

  @samandavariPosted about 3 years ago

  @pikamart thank u so much for your useful tips.

  0