Fully designed the calculator - Adir Avraham. #13
Conversation
|
|
||
| .form__number-input { | ||
| padding: 6px 12px; | ||
| font-size: 20px; |
There was a problem hiding this comment.
Because the input is Number, make sure the 2 up\down arrow buttons are not under the Dollar ican or under the Icon the Number of Peoeple.
You can use more from the padding to achive this:
padding: 6px 12px 6px 30px;
It will put the arrow buttons on the left of the icon.
| <div class="inputs-section__tip-section--tip-selection-group"> | ||
| <button class="btn" value="5">5%</button> |
There was a problem hiding this comment.
Good technique of Naming conventions in class names,
but try to avoid such a long names.
index.js
Outdated
| }); | ||
|
|
||
| const onCustomPercentageChange = (e) => { | ||
| percentageValue = e.target.value; |
There was a problem hiding this comment.
Make sure to decalre it as a const variable.
Think maybe to parse it, to check if its a number.
currently, if i put "abc" (letters and not numbers), i get Nan (not a number) error in the tip display box.
const percentageValue = parseFloat(e.target.value);
| } else { | ||
| toggleInputError(e.target, numberOfPeopleErrorMessage, ""); | ||
| numberOfPeople = Number(numberOfPeople); | ||
| calculateTip(); |
There was a problem hiding this comment.
I don't see the tipping per person in the calculation. i only get the calculation for one person.
i thing you should add
numberOfPeople = Number(numberOfPeopleValue); (and not for Number(numberOfPeople))
Something doesn't work here, check this please
There was a problem hiding this comment.
Thank you for the code review.
There's something I didn't understand.
What should I show on the summary list?
Tip amount - Is it the total tip? or how much each person tips?
Toal - is it the total price, including tips? Or the total tip?
There was a problem hiding this comment.
Tip amount = how much each person tips
Total = is it the total price, including tips, for each person.
| percentageButtons.forEach((button) => { | ||
| if (button.value === e.target.value) selectedPercentage = button; | ||
| }); |
| const percentageButtons = document.querySelectorAll( | ||
| ".inputs-section__tip-section--tip-selection-group button" | ||
| ); | ||
| const customPercentageInput = document.querySelector( | ||
| "#custom-percentage-input" | ||
| ); | ||
| const fifteenPercentageButton = document.querySelector( | ||
| '.inputs-section__tip-section--tip-selection-group button[value="15"]' | ||
| ); | ||
| const billInput = document.querySelector("#bill-input"); | ||
| const numberOfPeopleInput = document.querySelector("#number-of-people-input"); | ||
| const tipAmountLabel = document.querySelector("#tip-amount"); | ||
| const totalPriceLabel = document.querySelector("#total-price"); | ||
| const billInputErrorMessage = document.querySelector("#bill-error-message"); | ||
| const numberOfPeopleErrorMessage = document.querySelector( | ||
| "#number-of-people-error-message" | ||
| ); |
There was a problem hiding this comment.
Good declaration of variables.
Clear and understandable!
| <p class="list-item__labels-group-header">Tip Amount</p> | ||
| <p class="list-item__labels-group-caption">/ person</p> | ||
| </div> | ||
| <p class="list-item__total-price" id="tip-amount">$4.27</p> |
There was a problem hiding this comment.
Make sure text is 0.00 in the init of the site
| .calculator-box { | ||
| width: 100%; | ||
| margin: 50px 0 0 0; | ||
| border-bottom-left-radius: 0; | ||
| border-bottom-right-radius: 0; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| gap: 32px; | ||
| } | ||
| } |
No description provided.