🧮Tip Calculator | HTML, CSS | Javascript | Anime.js | Webpack

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. That animation is cool to be honest, haven't seen anything like that so great. The layout in desktop looks great as well and the mobile layout is great as well.

  • For the tag, it is fine to use header on this one since it contains the website's logo.
  • For javascript, I don't really dive into js when reviewing a site, unless there is a specific question about how they used js or a part.
  • I haven't use webpack:>

  Some other suggestions would be:

  • I would rather use img instead of object, if there are element that do a specific task that you want to do, use that element.
  • if that were an img, you need to use alt="splitter" , a website logo should be named as the website's name. I haven't use object to be honest, but using aria-label="logo" for the logo is not good. Avoid using words that relates to "graphic" such as "logo, icon, image.." when describing an image, since it is already one.
  • Theme toggle should be using button so that it will be more explicit instead of using div with tabindex.
  • If you are going to build a color theme, using input type="radio" are the ones that you should use. Since it is a selection, radio buttons are intended for those. Those radio button should be inside a fieldset along with a legend which can be hidden or not, depending on the design. Using this kind of structure, it is more accessible for all users. Have a look at my sample snippet about using radio buttons.
  • svg on the theme toggler should be hidden so use aria-hidden="true" on it, if you are using an svg and it is meaningful, use an title element inside the svg along with an sr-only class on it.
  • You don't need to use aria-label on the bill input since it already uses a label associated with it.
  • Using label that associates with a div will not work. So this element <label for="input--percentage" calss="input__label">Select Tip %</label> won't work since it points to a container div. label are for input.
  • No need for aria-label for each button since the text select tip 5 gives the overview of what the button will do. But to be honest, I won't use button for this, instead I will use input type="radio" inside of a fieldset since button alone is not accessible, unless you make like an aria-live element, that announces that the button has been selected or pressed.
  • Again, aria-label not needed for the next input which is the number of person.
  • Using ::after to hold the can't be zero is not accessible for NOT sighted users, since there will be no text-content that their assistive tech will read. It would be great to make use of a span for example. The span will contain the error-message and it will have an id attribute. The id will be referenced by the input element, if the input is wrong, that is where you add aria-describedBy attribute on the input and the value of that attribute, will be the id of the error-message. This way, users will know what kind of error they had made. Also, when the input is wrong, add an aria-invalid="true" attribute on it, so that users will know their input is wrong.
  • Avoid using multiple h1 on a single page. On the result section, you don't need h1 or heading tag to wrap the result of the calculator. The result will be better to just use p tag, since the Tip Amount is already a heading, giving overview on what the section will contain.
  • An addition as well, for the reset button, creating an aria-live element, that announces the calculator has reset. Since button does not inform that much, you kind of uses this kind of methods to inform users.

  Aside from those, great work again on this one.

  Marked as helpful

  2

  ---

  

  bunee•2,020

  @buneeIsSloPosted about 3 years ago

  @pikamart, I really appreciate you for taking the time out of your day to provide me some constructive criticism. I've made a note of all the tips and ideas you've given me and will be sure to implement them in my future projects.

  I'd like to mention the reason behind why I chose to use the object tag instead of the img tag. The SVG stroke animation that you see in the beginning of the intro requires the path of the SVG, and the path can only be retrieved if the SVG is linked in an object or an embed tag.

  I'm extremely thankful for your feedback. Thank you! Happy Coding :)

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @buneeIsSlo Ooh, I don't really have an idea about how that works so it is my first time hearing that :>.

  Thank you as well for that information and happy coding as well!

  1
• 

  Ctrl+FJ•810

  @FlorianJourdePosted about 3 years ago

  A piece of art !

  1

  ---

  

  bunee•2,020

  @buneeIsSloPosted about 3 years ago

  @FlorianJourde no u

  1
• 

  Paras Sharma•700

  @imparassharmaPosted about 3 years ago

  I was looking to your code and saw that you never use height. I mean how ...for the main container I have to define some height because without it then I cannot design my inner components accordingly. So can you tell me should I use vh in my body or main containers? Or how to proceed? https://imparassharma.github.io/Price-Component-Toggle-sections-/

  This was the last project I did on frontendmentor but took me a lot time to adjust heights for 1440px dimension. Can you help me!

  1

  ---

  

  bunee•2,020

  @buneeIsSloPosted about 3 years ago

  @imparassharma, Well this is something I struggled with too. A simple and short answer to your question is to use padding. As you can tell from my solution there's no need for defining the height of an element, doing so only causes unwanted trouble and you'll end up having to rewrite your code a bunch of times.

  I took a look at your code and noticed that you used margin-right and margin-left to center most of your elements. A better way is to use margin: 0 auto. This will not only align the element to the center but also make it responsive.

  I strongly recommend This free course by Kevin Powell. This course will definitely help solve most of the problems you're facing right now. If you don't have the time to sit through a course... In This video he goes over some of the topics from the course.

  Hope this helps :)

  0