Request path contains unescaped characters
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

tip-calculator-app-main

Dwi susilo• 325

@dwi312

Desktop design screenshot for the Tip calculator app coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


I'm studying javascript, please give feedback.. thanks

Community feedback

Raymart Pamplona• 16,090

@pikapikamart

Posted

Hi, the layout in desktop is good, for the mobile, it could use bit of paddings so that it won't touch both the sides of the screen.

The functionality works as well.

Suggestions would be that:

  • Your choice of radio buttons are correct, but its state is not properly used. If you used display: none on an element, especially for interactive elements, screen reader won't have any idea on how to use it, in this case, your tip will not work properly for those people. What you could have done instead is that, do not use the display: none property, instead just use a sr-only class on the input type="radio" then, on your css, you could have just triggered a input:focus-visible + label selector, then just put a outline property on the label so that keyboard functionality, as well as screen reader will work properly. I would remove remove as well the checked property that you declared on every input. When using input type="radio" checked property will only work with one, as long as the name property of the input is the same.
  • add a event.preventDefault on your form so that it won't refresh the page when a user presses the enter or click the reset button
  • header tag here is not really well suited since your image does not have any alt, add a text on it so that your header will function properly via landmarks in screen reader. The alt text could be the text from the image.
  • add a h1 tag on your solution. This h1 should have the sr-class. When I say sr-class, I mean that it should have screen reader only text, you could search for that one. Your h1 text should be the name of this app, then a little bit text to describe, maybe 4 words or so.
  • aria-live could be really helpful as well. This is addition if you want to really make your solution accessible.

Other than that, great work.

Marked as helpful

0

Dwi susilo• 325

@dwi312

Posted

@pikamart Thank you for the input, I will study your input

0
Hafizan Adli• 1,180

@hafizanadli

Posted

  • for better folder structure, you can make styles folder for your css file and scripts folder for js file. Instead of putting that file inside assets folder. Assets folder is for static file.
  • better make function handler for reset button, instead of reset by reloading whole page
0

Dwi susilo• 325

@dwi312

Posted

@hafizanadli naaahhh... apparently this is what I missed, I haven't given the settings to the reset function

0
Hafizan Adli• 1,180

@hafizanadli

Posted

@dwi312 woke semangat!

0
Dwi susilo• 325

@dwi312

Posted

@hafizanadli lah kok.. ternyata orang indonesia ya

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord