Roman numeral project

Hello, all.

I decided to go ahead and make a simple page to go with the Roman Numeral Calculator project and I just put it up on CodePen. If you have a moment to check it out, I would greatly appreciate any feedback you would like to share.

On a side note, it seems that the first entry is always transient on CodePen so please don’t give up when you notice that. For some reason the first one disappears and the page behaves normally thereafter. If any of you can tell me why it is behaving in such a way I will be very glad to know!

Here’s the link: Roman Numerals.

Nice job! Always nice to wrap a fun algorithm in a pretty UI.

        <form action="#" onsubmit="return convertToRoman(numberInput)">

I’d suggest removing the inline event handler here, because:

  • Inline JavaScript in HTML attributes is largely considered bad practice. This article gives a good overview of why.
  • Returning a value from an event listener does nothing. The return value isn’t used anywhere.
    • …With the caveat that in inline event handlers, return false prevents the default event from firing…
    • …But we just said we shouldn’t use inline event handlers. Which is a shame, because…
    • …We really want to prevent the form from submitting. The submit event is what’s causing your bug, because it causes the page to navigate away.

So how do we prevent it? Instead, you can use the preventDefault method of the event in your JavaScript, like this:

- submitButton.addEventListener('click', function() {
+ submitButton.addEventListener('click', function(e) {
+   e.preventDefault();

Preventing the default click action of a submit button has the same effect as preventing the default submit action of its parent form.

Now we have another problem — preventing the default event also prevents validation from happening. The validation wasn’t fully functional anyway, hence why you needed this message:

Results may be unpredictable outside of the suggested range.

…but at least it was showing messages if the input was invalid.

Let’s fix that.

form elements have a handy reportValidity method that can be called whenever you like. We can add this in to our click event listener:

+   e.currentTarget.form.reportValidity();

Better still, that method returns true or false depending on whether the form is valid:

-   e.currentTarget.form.reportValidity();
-   convertToRoman(numberInput.value);
+   if (e.currentTarget.form.reportValidity()) {
+     convertToRoman(numberInput.value);
+   }

Despite being called in an if statement, the side effect of showing the HTML5 validation messages will still happen if the form is invalid, so you can imagine it as if there’s an else displayValidationMessages() or something.


Thank you so much for the comprehensive response and thoughtful insights. I will be going over your reply in depth and working on assimilating the knowledge as I fix the page up.

Many, many thanks!

1 Like

I followed up on your suggestions and edited the code. Thank you again!