React: A number input will always have left pad 0 though parseFloat value in onChange

[HaydnChen]

Do you want to request a feature or report a bug?
bug

What is the current behavior?
I have a number input with defalut value 0 and in onChange function I'll parse value to float to avoid invalid input, but I'll always get left pad 0 on input UI. But in previouse version, my code works.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/84v837e9/).

What is the expected behavior?
Should not have left pad 0.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react 15.5.3, all browser / all OS. It works in 15.4.2

[gaearon]

cc @aweary

[aweary]

Thanks for the report @HaydnChen, I verified the bug issue with https://jsfiddle.net/84v837e9/ and have found the root cause.

[aweary]

To give some insight, the issue is caused by ReactDOMInput.js#L192-L200

// Simulate `input.valueAsNumber`. IE9 does not support it
var valueAsNumber = parseFloat(node.value, 10) || 0;
// eslint-disable-next-line
if (value != valueAsNumber) {
  // Cast `value` to a string to ensure the value is set correctly. While
  // browsers typically do this as necessary, jsdom doesn't.
  node.value = '' + value;
 }

If the input has the default of 0 and then, for example, the number 4 is entered node.value is updated to '04'. Since '04' is parsed as 4 it is not considered differnet and node.value is never updated. This check is meant to prevent issues with values like 0.00.

I'm not sure how we can address this just yet. We may have to do string-based comparisons on top of the existing check to determine if there are other cases like this where the node.value should be updated.

[nhunzaker]

This is really tricky. It looks like the standard browser behavior is to allow 04 as a value. We need to be careful to not make users feel like control is being taken away from them. The code @aweary points to also addresses the case of being typing 4e into a number input, which technically reports as '4' until you type another number after the e.

In Chrome, the number input reports values as follows:

• 04: "04"
• 4e: ""
• 4.: "4"
• -: "" (like before you type a negative number)

So it's tough, but I think there's something to doing a string comparison for left-hand zeroes.

@aweary I'd be happy to help dig into this. Or at least please let me know if I can answer any questions.

[aweary]

@nhunzaker I've got some work towards this locally so I'm happy to keep working on it. I'm hoping we could avoid relying on string comparisons but it's looking like the only way to fix this soundly 🤔

[antsmartian]

@aweary I'm trying to fix this issue, looks like the node.value update is happening over here (after the fiber changes): https://github.com/facebook/react/blob/master/packages/react-dom/src/client/ReactDOMFiberInput.js#L185:

if (props.type === 'number') {
      if (
        (value === 0 && node.value === '') ||
        // eslint-disable-next-line
        node.value != value
      ) {
        node.value = '' + value;
      }
}

However, I couldn't see anywhere we are parsing it to the Number like you have said above (like parseFloat etc). If the state of a input is 0, can't we add checks on this line: https://github.com/facebook/react/blob/master/packages/react-dom/src/client/ReactDOMFiberInput.js#L125:

initialValue: getSafeValue(
   props.value != null ? props.value : defaultValue,
)

where the initialValue is set? Will that help? May be a wrong idea. But shouting my thoughts.

[nhunzaker]

@antoaravinth Just for some background, there was a change in December of last year:
https://github.com/facebook/react/pull/11751/files

Parsing was removed, because we don't need it. A loose equality check (with some guards) is sufficient, because it parses string values into numbers for comparison.

[antsmartian]

@nhunzaker Thanks. I had found a way to fix this issue, the block of code now looks like the following:

if (value != null) {
  if (props.type === 'number') {
    if (
      (value === 0 && node.value === '') ||
      // eslint-disable-next-line
      node.value != value
    ) {
      node.value = '' + value;
    } else if (value !== 0 && node.value !== '' + value) {
      node.value = '' + value;
    }
  } else if (node.value !== '' + value) {
    node.value = '' + value;
  }
}

I would say this handles the case and fix the issue. No other test cases got failed and the issue was resolved. Yes, it sounds bit hacky, but I saw similar check was added for handling changes on 0.00 etc. May be @aweary can answer this. Thanks for your time on this.

[nhunzaker]

Interesting. It might be a bit hacky, but, given what some browsers give us to work with, it's the situation we're in.

This might be unit testable. I'd like to take these next steps:

1. [ ] Attempt to create a unit test for this.
2. [ ] Create a DOM test fixture
3. [ ] Do browser testing. If it turns out browser behavior is consistent, we can probably remove the manual fixture.

[antsmartian]

@nhunzaker Sure, will do that. Actually I have done the browser testing. After this change, ran build and used the dist version of react.js and seems to be working fine with chrome. May be will test in other browsers also and update/create new test cases.

[nhunzaker]

Awesome. Just to make sure it's surfaced, our DOM Test Fixtures are here:
https://github.com/facebook/react/tree/master/fixtures/dom

We have a section specifically for number inputs, if that helps testing. Once we move forward on a fix, my plan is to test it against those fixtures.

@antoaravinth: do you mind if I assign you on this issue?

[antsmartian]

@nhunzaker Never mind, you can assign me the issue. Will get it done :)

[nhunzaker]

Ah I can't assign you formally. But consider it yours.

[antsmartian]

@nhunzaker Sure, thanks!

[nhunzaker]

Just checking in! No rush on progress, but anything I can do to help move things along?

[antsmartian]

@nhuzaker: Hey, here is the update:

1. Create a DOM test fixture
2. Do browser testing. If it turns out browser behavior is consistent, we can probably remove the manual fixture.

For the above two points, I guess its taken care. After the change, I ran a sample app against the build and the issue is fixed (tested in chrome and firefox). I will write a unit test case for the same, so that we have test in place for this use case and raise a PR. What say? Sorry for the delay btw.

Mostly will close this by weekend.

[nhunzaker]

No need to apologize! Thanks for the update!

[nhunzaker]

@antoaravinth's fix in #12451 definitely addresses the concern of this issue. However I didn't fully understand the ramifications of this change until I tried it for myself:

Change in behavior

Before

Number inputs respect values if they parse to the same numeric value. So 00001 is the same as 1, maintaining the existing leading zeros in the UI.

After

Number inputs always use the reduced value. So if you typed 00000 then 1, the number input would reduce the text value to 1.

Concerns

This change makes it easier to control the absolute value sent into a number input, but I am concerned that it results in a confusing user experience. The key strokes a user enters into a text box do not align with what they see on the screen. Additionally, number inputs do not give us a text selection API, causing text selection to jump very easily. For example:

Deleting the leading zero in 0.* causes a cursor jump

1. Type 0.01
2. Delete the leading zero
3. Cursor jumps to end because the input's value is replaced from .01 to 0.01, inserting a character.

Inserting a number before the decimal place on 0.01 causes a cursor jump

1. Type 0.01
2. Move text selection to after the first 0 (0 [cursor] `.01)
3. Type 1
4. Cursor jumps to end because the input's value is replaced from 01.01 to 1.01, deleting a character

So what do we do?

Personally, I think we should optimize for avoiding character deletion while the user is typing. It might be possible that we could apply this behavior only when the number input is not in focus. That would allow you greater control over the value if changing it outside of user text entry, while avoiding confusion during typing.

What do you think?

[fiedlr]

Something similar happens with parseInt when your default value is 0. It doesn't let you erase the zero, which would be natural if it was erased on its own when you start typing some other digits but it just stays there even though it isn't present in the model (jsfiddle).

I've noticed it's already been referenced here too but as far as I know it hasn't been fixed so if somebody is still looking for a quick solution that doesn't have to parse through the value, I've fixed it for integers by interpreting an empty string in the view as 0 in the model (and vice versa):

<input type="number" value={this.state.value || ''} onChange={this.handle.bind(this)} />

Although it's still possible to have leading zeros this way when you try hard (first typing non-zeros and then typing leading zeros), that's not a real issue with integers. The number is saved correctly as long as it can be parsed by parseInt otherwise it's saved as 0. The only disadvantage I see is that you can never type 0 as the first digit because then the input's value falls back to '', which can be confusing for the user.

[Towerss]

Hi guys, any updates about this issue? I am testing in Google Chrome and I still cannot delete the 0. Once I change the focus from the numeric input the state of the application is updated to 0. If I type more numbers to the right of the 0 the number is saved as the integers > 0 that I entered, but the numeric input still shows the number with a 0 preceding, which is confusing for the user.

[JHanLu]

I change type = "number" to type = "text" and solved the problem.

[crimson11]

My solution to this is now quite simple:
If I do have a <Input type='number'> and the property of my object, which backs the value attribute of the Input has a js type of number, I simply add a second property, which returns the value of the original property as a string type, which I then use in JSX.

class SampleFormData {
   constructor() {
      this.myNumberProperty = 0;
   }
   get myNumberPropertyAsString() {
      return this.myNumberProperty + '';
}

<Input type="number"  value={formData.myNumberPropertyAsString}/>

In the onChange-handler, I make sure, to update my source object property (SampleFormData.myNumberProperty) with a number-type.

With this approach I am in total control:
I can use type="number" (and get browser validation for that) and also get removal of weird stuff like leading zeros, if I don't like it (and can accept cursor jumps on insertion).