Completed the tip calculator app challenge from frontend mentor website

https://pitstop260.github.io/splitter-akshay-koti/

Above is link to live site i created feedback appreciated. Thank you in advance :blush:.

2 Likes

Hello!

I would change the order of the inputs on mobile or assume at least 1 person (which should always be the case anyway): when you input the bill amount, the next step is the percent of tip and then the number of people. If you follow that order, it will fail because the people quantity to split the tip is required. In other words, make it more intuitive.

Other than that, it’s nice :slightly_smiling_face:.

Thank you for the feedback.

Yeah you are right the order is non intuitive, but i don’t know if i can change the order :grimacing:because the challenge had mentioned to make the app in this order so, I will assume no. of people to be 1 .

1 Like

Good job. I hope in the future you add the ability to add amounts that contain decimals

Nice, I am working on this very same project and I am having a hard time placing the dollar icon and the person icon inside the input field, by the way you can remove the step arrows of the input fields using this code in your css stylesheet:

input::-webkit-outer-spin-button,
input::-webkit-inner-spin-button {

-webkit-appearance: none;

margin: 0;
}

you can also assign placeholders inside the input fields using the placeholder property, this way when the user has not selected the input field or placed any text a grayish text can appear, placeholder=“0” is the sintax., this has to go inside the input element in the HTML document

I am also learning to code, and only been coding for around a month, do you have any advice on how to place the icons inside the inputs?

1 Like

This looks very nice indeed. I’m not familiar with Frontend Mentor and I don’t know the requirements of this project, so some of the things I suggest below may not be applicable. I’m not trying to be too critical here, I always try to suggest accessibility enhancements when I can because that is my current area of interest. Some suggested fixes:

  • The only action that seems to update the display is selecting a percentage. I think changing the bill amount or number of people should also trigger an update as well.
  • Having the 15% percent button remain a different color than the rest, even when it is not selected, is confusing. I think the currently selected percentage should be the off color. You might also think about displaying the current percentage selected in the brown display panel.
  • You have broken keyboard accessibility on the bill amount, number of people, and custom percentage inputs. When I tab into them my keyboard basically becomes useless because I cannot type a number, use the arrow keys, or even tab out of it. This is known as a keyboard trap. I think you need to fix the keydown event handler for these. You do not want to throw away key presses that you don’t intend to specifically handle (at least not without a very good reason). You should return true instead of false in this case. Returning true allows the event to bubble up so it can be handled by the browser as normal.
  • The placeholder text on the custom percentage input is not always doing its job. Depending on the browser width, you can’t read the entire string in the first place and placeholder text is not a substitute for a label (which this input does not have). Now maybe the design requirements don’t allow you to put actual text on this input? If so then I can’t blame you there but just know that it is very poor and inaccessible design.
  • Speaking of labels, yours aren’t quite correct in the HTML. Each label needs a for attribute that points to its input’s id.
  • For the percentage chooser, I’m not sure <button>s are the right way to go about this. Since these choices are mutually exclusive I think radio buttons grouped in a fieldset is probably the accessible way to go here. You can still use some trickery to make them look like buttons, but they would really be radio buttons underneath. I will admit that I’m not 100% certain about this suggestion. You could very well get away with buttons here but you would definitely need to add some aria attributes to inform assistive tech. which one is currently active.
  • I think you need to round off the amounts in the display. If I do something silly like set the amount for 1 dollar and number of people to 11 and then select any percentage the amounts in the display overflow the box.

Again, I know this is a lot and I really don’t mean to be too critical here and I’m not expecting you to make all of these changes. I’m just trying to point out that development is more than just making it look like the design. Accessibility should be a consideration from the start and you should always test your projects at the bare minimum to make sure they can be used with the keyboard only.

1 Like

Sorry for late reply, I was busy with my college practical exams :sweat_smile:. Thank you! for the feedback , I implemented your suggestion to remove the step arrows.

You probably might have figured it out by now but in case if you haven’t, This is the code i wrote

 HTML
 <div class="input-container">
  <i class="fas fa-dollar-sign"></i>
  <input type="number" id="bill" class="right" min="0" />
 </div>
 CSS
 .input-container {
  position: relative;
 }

 .input-container i {
  position: absolute;
  height: 23px;
  width: 25px;
  left: 0.5px;
  padding: 0 5px 0 5px;
  top: 10px;
  background-color: hsl(185, 41%, 90%);
  color: hsl(183, 100%, 35%);
 }

Thank you!

Sorry for late reply, I was busy with my college practical exams :sweat:, And thank you very much for such detailed feedback.

  • Fixed this issue, now both bill amount and number of people trigger the update as well.
  • Fixed this as well now all buttons are of same color and only change when selected.
  • Thank you for this suggestion learned some new stuff :blush: , fixed this issue as well.

  • This is the design they asked us to make , they have custom as placeholder i think
  • I have put for attributes now.
  • I tried using radio buttons but it messed up my whole css :sweat_smile:, i had put aria attribute aria-valuenow but html validator showed me error so, please suggest me any another solution.
  • Fixed this issue too, now we get values rounded upto 2 decimal points.

Thank you again, And please do suggest me some youtube videos on accessibility :blush:.

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.