
Mariusz Olszewski
@molszewski34All comments
- @vBenTecSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Hey i see few problems.
Only first 2 menu links have hover effect. Hover effect can look better if you lower transition time. Cart when empty change side. I had the same problem and for me the best and easy solution was to make a container with button and sum visibility:hidden; Cart in mobile view has big white space. Plus and minus buttons are too small. You can make them small but it is a good thing to put them in span so area in which you can click is bigger. I didn't test it on smartphone but buttons can be hard to push. Same thing with slider buttons. Button in cart do not always hover when i move cursor. Input number should keep the value visible when submit.
Marked as helpful0 - @jaronimas-codesSubmitted over 3 years ago@molszewski34Posted over 3 years ago
You can try update it with parseInt method and use toLocaleString to give value a proper format.
sumOutput.innerHTML = "$" + (sumOutput + parseInt(noRewardInput.value)).toLocaleString("en-us"); sumOfBacker.innerHTML = (sumOfBacker + 1).toLocaleString("en-us");
let sumOutput = 89914; let sumOfBacker = 5007;
Marked as helpful0 - @jaronimas-codesSubmitted over 3 years ago@molszewski34Posted over 3 years ago
- Why you want to connect them when they are totally separate things? Things that should connect are modal number input and sum of pledged money and counter of backers which add +1 every time value of input is added to the sum. You can also try to update progress bar when sum is updated. I used <progress> but it seems to not look properly (no styling) on android devices (atleast mine) but value is updated. Another option is to connect pledge options with modal options so when you click button an option in modal is also checked.
- You can try use a grid but there is also a way to use it with flex. Instead of <ul> you can make 3 separate divs with content and give them 33% width and then give them proper borders or even give borders to middle box.
Marked as helpful0 - @AyllaChristinneSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Hey. Solution works. Every calculations works as intended. You missing two things. When "person input" is 0 or less it should show alarm state - red border and text. Second is button which must show disabled and active status.
In matter of responsivness you could consider use of grid. This should help you a lot.
Good job!
0 - @luizclaudiocostaSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Still didn't get an answer about this, but one thing concerns me right now. Is us-eu currency format for input even a good thing to set in hmtl. Comma is another sign for javascript which can disrupt a whole calculation when not used with some methods. So you probably need javascript for this anyway which is not an answer for question :)
Marked as helpful0 - @luizclaudiocostaSubmitted over 3 years ago@molszewski34Posted over 3 years ago
I don't know but i will check if there is any easy way to do it w/o js.
Marked as helpful1 - @luizclaudiocostaSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Hey. It is one of better aproaches to what should be provied inside input. Most of solutions goes with step="1" which is not real in every scenario when someone pays with cents.
But why you don't used input type number with step="0.1" which gives you the same thing with less effort and you can also edit those numbers in easier way ?
Also it is not must have but reset button should look active when there is any change in status.
Marked as helpful1 - @JasonYuYangSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Hey. You miss one small feature. It is not so important but without it project is not complete. When reset button is used it should change style to darker to look as it is disabled and when there is any change it should look active again.
0 - @mjmorrison10Submitted over 3 years ago@molszewski34Posted over 3 years ago
Yes. I see that span when in position: absolute moves over input and block it.
0 - @imparassharmaSubmitted over 3 years ago@molszewski34Posted over 3 years ago
List of issues.
- Both input number move below zero which can't be allowed.
- Instead of calculate button you can try to add addEventlistener for a whole form so it will react to changes in inputs.
- Reset button not working on firefox
- You should lower your media queries because right now it shows mobile view for desktop when typical standard for laptops is 1024px max.
- There is no alert when input is zero or lower.
Marked as helpful1 - @kevinvan1989Submitted over 3 years ago@molszewski34Posted over 3 years ago
List of issues:
- Input number can't be allowed to move into minus. It is possible in bill case if someone has debt but there is no way for -9 persons.
- Input number when 0 or less should show alert.
- Calculations are not working for some reason on firefox. I checked it on chrome and it is ok.
- Custom input not working or working in way that is not intuitive for user.
- Reset button should show disabled status when used and active when there is any change.
- No hover effect over any button.
Code isn't too big and too complex. You code the way you can and with time you find better practice or way to make it shorter but it come with time... so don't worry and keep coding.
Marked as helpful0 - P@KatherineEbelSubmitted over 3 years ago
- P@KatherineEbelSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Hey. Almost everything is fine. The only issue is found is that when you change custom input from any value to 0 it keeps old value when it should show default value mb. 5% ? It is not so important but keeps this project from perfection :)
Marked as helpful0 - @mjmorrison10Submitted over 3 years ago@molszewski34Posted over 3 years ago
Hey. Calculation part works but there are few issues you should work with.
- Output should show .toFixed(2). This is required to show proper amount of currency. Right now when user provide 100$ for 3 persons it shows 33.333333.
- Reset button should show active status when user use number input and be disabled after use of reset function.
- Alert must be shown for person input and has red border when condition is false. 4.There are problems with responsiveness. Some elements as reset btn and output move out of fields and tip amount is not shown on lower resolutions. Especialy with big output. Fixing issue number 1 should also fix some of problems.
- It is ok to start function by clicking option button but its better to start it with one of inputs - person field or bill.
Checked on firefox and chrome.
Marked as helpful0 - @bjaneczkoSubmitted over 3 years ago@molszewski34Posted over 3 years ago
Looks good. The only thing i would suggest is to change reset button so it does not show "disabled" status non stop. Reset button should look active only when any input has been provided and disabled after you use reset and not provided any input.
1 - @molszewski34Submitted over 3 years ago@molszewski34Posted over 3 years ago
You mean that lower background picture in mobile view or something else?
0 - @omeizahanifSubmitted over 3 years ago@molszewski34Posted over 3 years ago
I hope this aproach will give you some insight. There are better ways to make it but it is one of simpler to understand.
form.addEventListener("input", () => { if (numPeople.value <= 0) { document.querySelector(".people__error").style.display = "block" numPeople.style.borderColor = "red" } else if (bill.value <= 0) { document.querySelector(".error").style.display = "block" bill.style.borderColor = "red" } else { document.querySelector(".error").style.display = "none" numPeople.style.borderColor = "green" document.querySelector(".people__error").style.display = "none" bill.style.borderColor = "green" } })
Marked as helpful0 - @Menna-RashadSubmitted over 3 years ago@molszewski34Posted over 3 years ago
In my aproach i used it for form. If you use input instead of submit it use functions anytime you pass the numbers so this way you get a real time calculation like in normal calculator.
If you are worried about alert msg you can try to give your input in html vallue=1 or give min=1 so this way it should never show alert msg at the start but keep in mind this is reserve for input type number. I used input type number.
I checked alerts again and now it works.
Marked as helpful0