Mobile first page with CSS, JS, BEM, Semantic HTML5

Community feedback

• 

  Sean Dickinson•135

  @atinybeardedmanPosted over 3 years ago

  Hi there, I think there's a few things you can do to improve this one. I wouldn't say the goal is always conciseness, but rather readability. Concise is great, but being able to read and understand code is better. So here are some suggestions/general feedback:

  • In javascript try to avoid comparing things directly to boolean values in a condition. Instead just use the boolean directly. So in your case you can just check if(checkedValue.checked){...}. Along those lines I would consider a clearer variable name. So in your case the "checked" state means they want monthly amounts. So I would name it something like isMonthlyCheckBox or something like that so it's super clear what the logical value is associated with. Also in here you have a minor bug with the array indexes. You are making the pro plan cheaper than the basic plan in the js.
  • I think your css BEM style looks really good here. You have clearly distinct namespaces for the different sections they are easy to read and understand.
  • Your html markup looks good. These buttons don't do anything so it's a toss up if they should be buttons or a tags, but you have aria role button on the a tag which might actually be a bit confusing to screen readers since they already pick up a tags on their own. I might just stick with an a tag without the role button, but I'm not an accessibility expert.
  • You have a minor reflow issue when you hover the middle button on desktop. This is because the border doesn't exist on the non-hovered state, so the 2px that is added (1 for top and 1 for bottom) is enough to visibly shift the layout of the card. Not a big deal, but easily fixed by adding a 1 px white border to all buttons so that it's always there are really you are just changing the color on hover. In terms of these color changes, I'd highly recommend adding a css transition on the btn class for color, border-color, and background. It's subtle but it brings up the percieved user experience a bit.

  Overall this looks great, nice work!

  Marked as helpful

  3

  ---

  

  Cássia•90

  @casbernPosted over 3 years ago

  @atinybeardedman Hi Sean! Hope you are fine! I want to thank you very much for looking through my code and for giving me such valuable feedback. I really really appreciated it.

  You were very detailed and that is all that I wanted and needed, cause I really want to improve. You even saw things that I did not and I looked a lot my code.

  I took you advice and changed my code. I will also exercise now how to use the transition, because for some reason I do not find it very easy...

  But I learned a few things that I did not know before with your feedback which is wonderful.

  So thanks again!

  0