@TimilehinEmmanuelOJO
Isabela Oliveira
@oliveridsAll comments
- @oliverids
That's probably because you coded it for the calculation to be done at every change in the input's value. You can fix this by only executing the calculating function after you notice the input isn't empty. An if condition sounds enough, something like "if(input.value.length !== 0)".
Marked as helpful - @NovidicusTitan@oliverids
Hmm I might be wrong but I tested it really quickly on DevTools and I think the issue is that you didn't access the element correctly.
Notice how, for the main style, you accessed the element by ".card .input .percent .percent_btn button { ... }". However, for the hover, you used ".card .btn:hover { ... }". Might be just a matter of confusion for the browser ? I changed it to ".card .input .percent .percent_btn button:hover { ... }" and it worked just fine, without the !important.
Might be worth it to give it a try.
- @NhanPhamTrong@oliverids
I think your scss organization looks just fine, but I think you could join the _color.scss and the _variables.scss into the same file. Though it isn't necessary, it's fine the way it is.
Your background-image property does not work because you aren't correctly accessing them. Sass compiler is literally just a compiler, it's not the final file. When you want to use an image as a background, you have to remember that you need to access them while you're inside the CSS FOLDER, because that's where the SASS files are being compiled into. Since your images are in a folder OUTSIDE of the css folder, you need to "go back" in the adress, and you do that by adding a "./" to it.
Your images can be accessed by: background-image: "./images/ ...".
Marked as helpful - @sccala@oliverids
Hey, I think part of the reason is that you put the style element for the theme change for the background on the html tag. I don't know if I can say it's wrong, but definitely not recommended. If you want to change the color of the background, add the "light" or "dark" class to the body instead and check again if it works.
Marked as helpful - @michaellosdev@oliverids
For the tips, for example, you can shorten the code by using if else with the help of an event listener on a list of all buttons. Your code would look a bit like this:
let buttons= document.querySelectorAll('.tip-select btn'); //this will create a nodeList
buttons.forEach(each => {
each.addEventListener('click', event => {
if(event.currentTarget == fivePercent) {
//
} else if(event.currentTarget == fivePercent) {
//
} ...
}); });
Also, adding the attribute "type=number" also shortens your code because it'll only allow the user to type numbers, and you won't need to do the verification "if(NaN ...)".
- @HualDev@oliverids
Hey, i think your question can be solved with an EventListener!
Add a listener for the "input" event on the bill input. The "input" event catches any and all changes made on the value of said input. After you've done this, you can make all functions work when the browser notices any changes.
Your code would look a bit like this:
let billinput = document.getElementById('bill');
billinput.addEventListener('input', () => { ** all the calculation functions ** )
Marked as helpful - @VladimirBrscic@oliverids
Hey, your code is really neat and works just great!
I'd suggest using the "input" event instead of "keyup" if you want a more imediate trigger, but that's not necessary if you don't want it.
Also, for better usability, I suggest not letting the calculator do its thing until the person types the number of people. Your code says that when the value of the input is zero, to use "1" as default value. However, a random user can't see that, and they might think there's an error. I think something like this would be enough for you:
if (input.value.length) { ..function.. }
Nice job!!
Marked as helpful
