Tip calculator app || using CSS and Vanilla Js

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted over 3 years ago

  Hello

  This looks good and works well functionally

  I'd probably use radio buttons for this as well tbh... what you've done still sort of works but isn't very clear.

  There are definite accessibility issues with this though. It's not clear from the document structure what's happening. I think this needs

  • to be a form
  • to have labels for inputs (e.g. Bill is really a label)
  • to have error messages linked to their inputs with aria-describedby and aria-live (make errors display none until they have content, or add aria-live attribute dynamically at the same time as adding that error message content)
  • Use more obvious and consistent focus visible styles (e.g. reset has no focus style)
  • style the buttons to show which tip is actually applied
  • use sections and headings or legends if needed in the form

  I think you need some body padding or margin on main to stop content hitting the sides at some screen sizes.

  It also looks like you're using margin and padding very inconsistently in this, adding it on individual elements in different directions. You could make all that a lot simpler with some forward planning...

  Good luck

  Marked as helpful

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted over 3 years ago

  p.s. Remember to remove console.logs ;)

  0

  

  soransh singh•1,025

  @soransh-singhPosted over 3 years ago

  @grace-snow thanks .... Your comment is Really helpful

  0
• 

  HYDROCODER•555

  @HYDROCODERPosted over 3 years ago

  Hey there! It looks and works great!. While I observed nothing wrong, I would suggest the following if you don't mind:

  1. Do put comments in your code. It will be helpful for anyone using or seeing your code as well as for yourself if at all you need to correct it again.

  2. There is an accessibility issue regarding inputs. You have used input elements without labels for them so screen readers won't be able to identify them. I would suggest to use labels instead of "para" divs for the headings and place it above the inputs. This way it will be more semantic, more accessible and less code in html.

  3. You have used buttons for tip percent input. While it does work out great and styling it is easier, I would suggest to use radio buttons although styling them is another headache to take care of. That way once the user clicks a tip percent, the background color stays as it is using :checked pseudo-class selector, as well as it would be more semantic and accessible.

  Hope it helps :).

  Marked as helpful

  0

  ---

  

  soransh singh•1,025

  @soransh-singhPosted over 3 years ago

  @HYDROCODER thankss... your comment is really helpful ... I will apply the changes you have suggested ...

  I thought of using radio button but I was not comfortable using them so I used buttons instead..

  Yhaa adding comments will be really helpful but I am just too lazy to add them ... I will start adding comments from next challenge

  0