[Time Input] is allowing NaN in the hidden value field.

[xenoworf]

Bug Report

Description

If you start with an empty Time Input and enter a single digit in the 2nd or last field (e.g. minutes in hour/minute or seconds in hour/minute/second) then the component is letting the string "NaN" through.

This is a bug because NaN shouldn't be a part of even a partial iso-8601-like value, which is supposed to be what that hidden field is.

Steps to Reproduce

You can go to the component's doc page and try what I've said above. Inspect the output in your console and you'll see the NaN.

Additional Context / Screenshots

The problem is happening because parseInt(val, 10) results in NaN when val is '' (empty string).

Here is a screenshot w/ the console showing a value containing NaN:

Expected Behavior

Certainly not seeing NaN in the value. There are a couple behaviors we could do instead. It all depends on what behavior we want when the user enters a partial value. We DO expect the consumers to take care of end-user-input validation - so we can't try to "fix" the value in the component.

Possible Solution

@benbcai @jmsv6d @neilpfeiffer

[xenoworf]

On the face of it, the two easiest options either:

1. Fill in "00" or some default time value (what?) for the other fields so that they'll parse correctly. This might surprise consumers and users.
2. Filter out NaN and do nothing else. This would result in values like ::01. Is that better?

[xenoworf]

I just chatted with @neilpfeiffer and we both agree the safest and most passive approach is to prevent the value field from being populated (having a value other than empty-string or undefined) unless the other inputs are minimally value, e.g. valid numbers.

I will probably implement this soon as a part of my a11y enhancements.