Responsive tip calculator app using Bootstrap and Vanilla JS

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Achintya, well done with this app! I think this is a good start, and I got a few suggestions on what you can try:

  About the positioning: I see you using a max-width: 50% on your container; this is actually causing your component to look small at around the 1000px break point with a lot of white space around it. I suggest not using 50% for the max-width, and if you do need to use a percentage, do keep it fairly large so that thing won't look super small. What I suggest is to use width: min() so that your component can resize and expand to a certain width without looking terribly small. Also, regarding centering, I think you can try centering it vertically at least in the desktop version. Here's how everything looks like:

  // desktop view
  
  // for vertical centering
  body {
    min-height: 100vh;
    display: flex;
    flex-direction: column;
    justify-content: center;
  }
  
  // for setting the width for responsiveness
  .container {
     width: min(100% - 6rem, 1000px);
  }
  

  The width: min(100% - 6rem, 1000px) declaration just tells the browser to choose the smaller value of the two options, which is either 1000px or 100% - 6rem (the 6 rem is for spacing around the component, like margin). For example, at a breakpoint of 1000px, 100% - 6rem would be something like 900px, and compared to 1000px, the 100% - 6rem option would be smaller, and so that's what the browser chooses. You can choose whatever values you want.

  About the functionality, there's a bit of a lag in calculation right now, and I notice that you added a setInterval; I wonder what's the thought process behind that decision. I would suggest something smaller than 1000 just so it can be a bit quicker, but maybe you have your reason for that.

  Lastly, instead of the innerHTML, I suggest using textContent instead because you don't actually have any HTML in the string that needs to be parsed, and also when it comes to inputs, it's always better to avoid innerHTML, even though the type='number' input already does a layer of validation and "cleaning". The main thing is innerHTML is a bit slower in repainting compared to other methods.

  If you're interested, here's my solution for this challenge!

  Great work here, and keep practicing 😊

  Marked as helpful

  1

  ---

  

  Achintya Keskar•40

  @SanePiePosted over 2 years ago

  @elaineleung Thank you so much for your suggestions Elaine. I will definitely play around with the changes you suggested and implement those in the final version.

  Also there was no thought process to the 1000ms interval, I just needed my main function to run after a certain interval and arbitrarily decided that a second would be fine. This is the max amount of js I have written up until now so even though running the main function every second seemed a bit wrong to me, I did it because I just wanted to complete the challenge ASAP because I was very frustrated (I blame CSS) :P

  Your Tip Calculator looks freaking awesome and is smooth af so I'll also definitely look through your code and try to understand the things you've done. You've been a great help. Thank you so so much.

  1
• 

  Achintya Keskar•40

  @SanePiePosted over 2 years ago

  I was having problems with centering because of inheritance as I didn't want the containers data to be centered due to the design. However now I added text-align:initial to the container and everything works fine. I also added margins and better media queries for the mobile view now. Thank you so much !! :)

  1
• 

  Divine Obeten•2,415

  @Deevyn9Posted over 2 years ago

  Hi Achintya, congrats on completing this solution, double congrats for it being your first junior challenge. However, you should center the body’s contents and it’ll be perfect.

  Happy coding 🎈

  1