Order summary component

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello there, Roberto! 👋

  I notice there are some invalid BEM class like order__summary__paragraph which makes it BEE (block__element__element) instead of BEM (block__element--modifier). So, I recommend creating new block element.

  I would strongly recommend having the "uniminified" stylesheet. This way, people can read your stylesheet and possibly give some feedback. For me, it's hard to one by one the Sass file and try to give feedback. So, having the unminified CSS in the repo is recommended.

  There's no need for aria-label if there's text content inside the anchor tag (the same rule applies to the button element). Also, for your attribution, choose either aria-label or sr-only text. I would prefer sr-only because aria-label is currently not translatable meaning if the users try to translate the page, the aria-label content won't get translated.

  That's it! Hope you find this useful! 😁

  Marked as helpful

  1

  ---

  

  Roberto Yukio Miyamoto Arita•320

  @RyukioMiyamotoPosted over 2 years ago

  @vanzasetia Thank you so much! I was actually struggling with this particular class and just let it be and ended up forgetting to fix it..😅 big mistake. I take it I did ok with the rest of the BEM classes since you didn't mention else? This is literally my second project using scss so I'm still getting the hang of it. Regarding the aria-label, I wasn't aware of this translatability issue so, dully noted! Thank you again for the feedback, have a great week.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @RyukioMiyamoto You're welcome! 🙂

  About the Sass, my point is to provide another version of the CSS where it is not minified so that people can easily see your stylesheet.

  Have a great week too! 👍

  1