Feedback for frontendeval mortgage-calculator solution

fee-mortgage-calculator-feedback.md

Feedback for frontendeval mortgage calculator question

I just want to preface this feedback by saying you did a great job on this question, and your write-up was fantastic! The majority of the following feedback here is along the lines of "if you weren't under a time limit in an interview / you were in a dev role and submitting code that would go to production".

• Good job using labels correctly associated with the inputs: this is critical for screen readers and other assistive technology!

• I liked your approach to formatting the currency: this is a really tricky problem that has a bunch of different possible solutions (number/string formatting is a pretty popular interview theme too). Thankfully for the web JS added a utility to do the heavy lifting for us: Number.toLocaleString. You can call toLocaleString on a number and pass in a locale (e.g. 'en-US' for United States) and it will format it correctly, e.g.

const foo = 123456.789;
console.log(foo.toLocaleString('en-US'));
// logs "123,456.789"

You can pass it a second parameter of 'options', including a style of formatting (e.g. 'currency'), the currency to use (e.g. 'USD'), and even a maximum number of decimal places to round to, e.g.

const foo = 123456.789;
console.log(foo.toLocaleString('en-US', { style: 'currency', currency: 'USD', maximumFractionDigits: 0 }));
// logs "$123,457"

• It's preferred to use <button type="submit">Your button text</button> rather than <input type="submit" value="Your button text" />. With a button you can have nested tags, so you could have a button with an image tag, or text wrapped by a span to style it slightly differently. Also a button is a bit more explicit (i.e. it's immediately obvious at a glance it's a button and not an editable input)

• Instead of using a click event listener on the submit button, you could wrap all the inputs in a form element and use a submit event listener, e.g.

<form id="mortgage-calculator">
 <h1>Mortgage Calculator</h1>
 <!-- rest of the form -->
 <button type="submit" id="submit">Calculate</button>
</form>

And for your JavaScript:

const formElement = document.getElementById('mortgage-calculator');
formElement.addEventListener('submit', function (event) {
  event.preventDefault(); // this is needed to stop the form POSTing and reloading the page (the 'default' action for an HTML form element)
  // the rest of your JS code
});

This approach is more 'semantically correct', but the big benefit is again to screen readers, assistive technologies, and even anyone using a keyboard: you can now press 'Enter' while focused on any field to submit the form

• If you use a form element, you can add further field validation into the HTML to offload some of the work your JavaScript is doing. You already have min="0" on your fields, but you can also add the required attribute to your inputs (e.g. <input id="principal" type="number" name="principal" min="0" required />). If you add this, you're safe to remove your if (principal.value > 0 ...) check in your JS

• Since you're never re-assigning them, your variables p, r, and n can be const rather than let

• I like to stay away from single-letter variable names (except maybe i when I'm tracking the index in a loop) to make it more easily readable by others (and myself 6 months later!), for example p=principalAmount, r=monthlyInterestRate, n=totalPayments, tot=monthlyMortgagePayment (Note that coming up with good variable names is HARD)

• Your CSS is really solid: great use of CSS variables, rems for font-size, media-queries for responsiveness, etc, and the styling page styling looks really good overall - you definitely went above and beyond on the question!