Mortgage Repayment Calculator

Community feedback

• 

  Yulia•400

  @YuliaLantzbergPosted 3 months ago

  Hi. It looks very similar to the design, except for the 'clear all' link and also I liked that you used the CSS variables. But I have some thoughts going through the code.

  1. Though "clear all" looks like a link, actually it acts rather like a button. So I'd use the button tag for it and style it like a link.
  2. Throughout the CSS code you are not consistent in using rems. I mean, in some places you use px and in some rems and it's no really any need to use px. It's better to stick to rems for better responsiveness and also better readability. To make rems easy to use, you can set font-size to 62.5% in the root element. So it will be 10px and all rems will be multipliers of 10 and then you will not need any complicated calculations to use it.
  3. I couldn't figure out this selector - input-group>div:nth-child(-n+3) - is it every div in input group that is smaller than the third one? Why did you use so complicated selector instead of class? It's hard to read and will be hard to maintain if application is growing or used as a component.
  4. Why didn't you choose the fieldset tag to group elements in the form. It's also good for accessibility and for structuring the form, I think.
  5. And in the js file, I see there are many variables that are not changing, but you still use let for them instead of const. For example the variables in the beginning of the file: let empty_result = document.getElementById("empty-result"); let completed_result = document.getElementById("completed-results"); let form = document.getElementById("mortage_repayement_form"); And actually most of the variables in there. It's a not good practice as I know because it increases the chances of getting bugs. Especially if another developer will work with your code. Hope my comment is more helpful than annoying ;-) Happy coding.

  0