Tip calculator app main with JS

Community feedback

• 

  Cats-n-coffee•670

  @Cats-n-coffee

  Posted over 2 years ago

  Hi Jack!

  Great job! Js isn't always easy to use on interactive UIs like this project. Feedback about UI/UX:

  • The error message for the number of people input never goes away. Maybe you can improve this by using a blur event or something similar. Also this same error seems to show when you only have a single digit bill amount, you might need to tweak your logic there.
  • Nice job with the responsiveness! It's only missing something like padding bottom on the mobile version, and the "Custom" tip input isn't readable when the screen is less than 1100px wide for desktops.

  Code feedback:

  • Try to clean up your code when you're finished with your project, in the future if you want to apply for jobs, having a clean codebase will be important. Removing console logs, commented code, ... are all easy fixes that change things in an instant!
  • nice job using inputs! I would suggested replacing h2 with label for "bill" and "number of people", this will improve accessibility and label is better suited for this.
  • Make sure you only have one h1 per page. h1 should be the main heading which is usually a company name or something like that. You can use as many of the other heading levels as you want (h2, h3, ...), but only one h1 per page is better for SEO and accessibility (and HTML semantics). I'm not sure the totals fit well the headings, maybe you can find a more appropriate tag here? https://developer.mozilla.org/en-US/docs/Web/HTML/Element
  • Great job using const, you can actually use it for all the elements you're querying, so you can replace let tipAmount with const tipAmount and same for total and percentCustom. You're never re-assigning the value, so they can be const.
  • I think you could break up your checkEverything function to isolate functionality. The tip calculation can be on its own as well as the people check with the error message.
  • You could place the reset logic inside its own function as well, just to make the code cleaner (though what you did works as well!).
  • I'm not sure why you place the checkEverything inside a setInterval? from my point of view you only need to run checkEverything on user input, so the event listener should be enough, but I might be missing something.
  • Overall, you run the same function checkEverything inside the event listener of each element. If you split this up a little bit more (which would make you code more modular), you can isolate logic to the part that really needs it.
  • line 22 seems like it's missing a space, not sure if that's causing unwanted behavior?

  Nice job, hope this helps, let me know if you have questions!

  02

  Marked as helpful