Design comparison
Solution retrospective
I paid a lot of attention to details. Results can display numbers with up to 8 digits without breaking the layout. The app displays error messages according to validation results. I believe JS is written according to best practices and is easily scalable. I have tried to use pure functions as much as possible.
Community feedback
- @webdevbynightPosted about 1 month ago
Some feedback:
- the reset button is displayed with the colours foreseen by the design for the state when it is disabled (this button has three states in terms of design: disabled, enabled and hovered);
- when I forget to choose a tip percentage, the error message “Can’t be null” is displayed, but when I correct the error, the error message does not disappear;
- when clicking on the reset button, not everything is reset: the tip percentage button I chose is still checked, for example (by the way, to implement a reset button, you can just use the
input
element with thetype="reset"
attribute and let the form behave as it does when resetting, just adding a listener to thereset
event to reset the amounts calculated to $0.00); - to improve the accessibility of the form when there are invalid inputs, you should have a look at ARIA (this article from Smashing Magazine can help you);
- the two icons used to decorate the two main fields of the form do not need an alternative description, since they are decorative (by the way, they can be displayed as background images in CSS);
- since you used TypeScript, you should avoid using the
any
type: theany
type tells TypeScript not to check the variable, which can be pretty unsafe; - the
as <type>
statement in TypeScript can help, but should be used quite carefully, since it is a kind of hack: for example, instead of declaringconst form = document.getElementById("form") as HTMLFormElement;
, you can do this:const form = document.getElementById("form"); if (form) { /* Here the instructions where form is typed as the non-null type infered by TypeScript */ }
, or this (in case you want to later use properties and methods that are specific to aform
element):const form: HTMLFormElement | null = document.getElementById("form"); if (form) { /* Here the instructions where form is typed as HTMLFormElement */ }
; - when using TypeScript, in your
.gitignore
file, you should add a line to ignore any.js
files generated by the TS transpilation.
I hope this feedback helps you.
Marked as helpful0 - @NikitaVologdinPosted about 1 month ago
Hi @webdevbynight! Thank you for your detailed feedback!
- According to the Figma file, it's quite difficult to say what the designer meant with the reset button. However, I like your suggested approach more than mine.
- For me, everything works. You just need to lose focus from the custom tip input.
- Thank you for noticing. I have fixed that and didn't know about reset type. Thank you for sharing.
- Thank you for the provided article. Indeed, I am very interested in accessibility and improving it with every work. I'm looking forward to my next learning path here, which is accessibility.
- That's a good idea for the background image for icons. However, my thoughts were about accessibility: how do users find out what kind of information to enter? I thought that headlines like "bill" might not be enough for everyone, so I decided to provide alt text for the icons.
- I used
any
for the user inputs, but after your notice, I realized that the entered inputs are always strings. - Yes, but I was sure that the elements I was looking for would be in my HTML.
- I don't understand why I should add JS files to my
.gitignore
. Can you explain pls ?
0@webdevbynightPosted about 1 month ago@NikitaVologdin Concerning the
.gitignore
feedback, since you use TypeScript to do all the JavaScript stuff, when pushing your basecode to deployment, generally this basecode will run a command to build the app, which will, among other things, transpile the TypeScript code into JavaScript (using the nativetsc
command or a bundler) based on the instructions withintsconfig.json
. To make Git ignore.js
files generated by transpilation from TS is not mandatory, but it will reduce the weight of your repo.0
Please log in to post a comment
Log in with GitHubJoin 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