@H4m4k
Posted
Good Job, congratulations on finishing the challenge.
Some points I would change :
HTML/ CSS
- class result has display grid on it, Bill Sum per person text does not fit inside the grid because of the size of grid lanes, try changing the size of the lanes. You have used grid-template: 1fr 1fr , which you can easily adapt to a more fitting sizes and prevent going out of bounds on some resolutions. .4fr and .6fr will do a much better job, because You split 1fr unit, which is much easier to understand and covers the whole class and split it into 40% of the fraction and 60% of the same fraction, which makes your Bill Sum
- border solid 1px in my opinion should be minimum 2px or .1em to instantly see the red
- as a rule of a thumb ,check your HTML in W3C validator and solve problems before posting :)
JS
- entering small bill like 5$ and 6 people makes your calculation display that it can't be greater than bill, I advise to revise your logic as its a extreme case but still within boundries of normal operation
- const removeSelect with anonymous function and a for loop to remove the class is a very hardcore way of solving an easy problem, just use a document.querySelector('.button.select').classList.remove('select') to remove the selection and preserve resources
- also I have noticed You are using a non strict comparison operator == which in JS can lead to strange behaviour, so I would advise against using that and use === strict comparison
- lastly You are using another for loop in a simple task of looping through elements, check the porcentage[i].addEventListener part - line 58. Instead please have a read about a high order functions - forEach is the one You should use instead of a for loop.
Cheers Martha
Marked as helpful
@Mtc-21
Posted
@H4m4k thanks for your comments, I use a validator html and css, as well as the WAVE tool helps me a lot, I corrected some parts, but other details came out that I will correct on the fly, again thanks for your comments