Order Summary Card | HTML, CSS, Flexbox

Community feedback

• 

  Byron•2,290

  @byronbyronPosted almost 3 years ago

  I agree with the above from David.

  I think the three <section> elements are a bit unnecessary. If anything, the whole card would be in a section, but I don’t even think that’s necessary, in this case. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/section

  1
• 

  P

  David Turner•4,150

  @brodiewebdtPosted almost 3 years ago

  Your solution looks good. You don't need to add all that @font-face code in the CSS. Go to Google fonts, select the typeface and the weights you want and put the link in the head of the document. Then just add the font-family code to the body selector. Another thing you should do is use rem and em units instead of pixel values. Rems and Ems are responsive, pixels aren't. Hope this helps.

  1

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  @brodiewebdt As far as I know, using a local font or @font-face has no bad impact on users. I agree that this might look overkill and a lot of code to write, but it has several benefits:

  • It allows the browser to choose the best and supported format possible, which now the woff2 is the best format. But the woff2 format is not supported by the older browsers like IE and old browser versions. So, the older browsers can choose different font file types.
  • (Might happen(?)), if for some reason the google fonts site is down, then the local font will save the day.

  0

  

  P

  David Turner•4,150

  @brodiewebdtPosted almost 3 years ago

  @vanzasetia I was looking at it as the amount of code to write. I can't speak to the load impact.

  0
• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  👋 Hi there!

  I have some feedback on this solution:

  • Accessibility
    • About the HTML markup, that's right what Byron has said. You don't need three section tags, since those section tags will be treated as div tags, which means that you can just swap the section with div tags.
    • For the @font-face, In my opinion, is OK. Since the modern browsers will only download the woff2 version, while older browsers will download another type instead.
    • 👍 Good job on handling all images!
    • Create a custom :focus-visible styling to any interactive elements (button, links, input, textarea). This will make the users can navigate this website using keyboard (Tab) easily.
    • Always wrap the text with a meaningful tag, such as p (in this case). Only use span or div for styling purposes.
    • I would recommend adding type="button" to prevent some browsers from submitting any information.
    • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs.
    • Don't limit the height of the body element, it will not allow the users to scroll the page if the page content needs more height. Use min-height instead.
  • Best Practice
    • I would not recommend using the non-standard CSS properties, since it has bad browser compatibility. Here, I'm talking about the zoom property. I'm afraid that different browsers may treat it differently which can lead to an unexpected result.

  That's it! Hopefully, this is helpful!

  0