solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.4k stars 924 forks source link

Number input resets when typing period on Firefox 113 #1756

Closed Tuoris closed 1 year ago

Tuoris commented 1 year ago

Describe the bug

Number input resets when typing period on Firefox 113.

Here is source code for minimal working example:

function App() {
    const [value, setValue] = createSignal("");

    return (
        <>
            <input type="number" value={value()} onInput={(event) => setValue(event.currentTarget.value)} />
            <div>Current value: "{value()}"</div>
        </>
    )
}

Your Example Website or App

https://tuoris.neocities.org/solid-input-number-minimal-example/

Steps to Reproduce the Bug or Issue

  1. Run minimal working example or open this page
  2. Focus on input
  3. Type: "4", ".", "2"

Expected behavior

Input value should be "4.2"

Screenshots or Videos

https://github.com/solidjs/solid/assets/16479460/0a285676-6475-4c9a-90b6-ae83bf350f76

Platform

Additional context

I've discussed this issue with Firefox team - looks like they are following W3C standard for valid floating point number

Tuoris commented 1 year ago

Similar example works in React (although onChange/onInput semantics is different from SolidJS), maybe these lines can be helpful to solve the issue - https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/client/ReactDOMInput.js#L118

titoBouzout commented 1 year ago

I can sympathize with the problem, but this is not a bug. The example code is doing setValue(event.currentTarget.value) which translates to setValue("") in firefox when the number ends with a ".". So Solid will do just that and empty the value of the number, which seems correct to me. You probably want to do event.currentTarget.value !== '' && setValue(event.currentTarget.value)

Tuoris commented 1 year ago

Hi, @titoBouzout ! Thanks, I've checked your workaround (you can try it here - https://tuoris.neocities.org/solid-input-number-minimal-example-workaround-by-titoBouzout/ ), but it is looses reactivity when I'm trying to clear the input or next currentTarget.value is "".

https://github.com/solidjs/solid/assets/16479460/7c75ab82-4898-449e-831b-4f4666522901

I still believe it is a valid issue, because:

  1. everyone who use SolidJS need to know about it and consider this when working with inputs with type="number". It breaks consistency:
    • type="text" - setValue(event.currentTarget.value) - works ✅
    • type="date" - setValue(event.currentTarget.value) - works ✅
    • ...
    • type="number" - setValue(event.currentTarget.value) - doesn't work ⚠️ - need client side workaround
  2. it works properly in React - so there is a frameworks-side solution

Meanwhile, I've found this useful property of an input element - MDN input.valueAsNumber, so here is my (!not ideal) workaround:

function App() {
    const [value, setValue] = createSignal("");

    return (
        <>
            <input type="number" value={value()} onInput={(event) => setValue(
                isNaN(event.currentTarget.valueAsNumber) ?
                    event.currentTarget.value
                    : event.currentTarget.valueAsNumber
            )} />
            <div>Current value: "{value()}"</div>
        </>
    )
}
titoBouzout commented 1 year ago

Didn't know of valueAsNumber. :)

It is still an invalid issue because if you think about it, you are just using a signal. There's no way for the signal to know that's an input of the type number, or that the data comes from an event, nor really doesn't care because you just do getValue and setValue anywhere with any kind of data.

I still sympathize, but the way I see it, if you start messing with events then you enter dark waters, you will end defining a whole new behaviour like the way onChange, onInput works in react.

atk commented 1 year ago

My preferred way is not to overwrite the number on every input, but let the browser manage the field. Using /*@once*/ or untrack(value), you can set the value only once on instantiation, and then just read the value from the event (the browser should ensure you have the same value in your field as in your state).

ledburyb commented 1 year ago

If not relying on browser form submission you may also want any validation logic to track the field validity state: https://developer.mozilla.org/en-US/docs/Web/API/ValidityState/badInput

mattbdc commented 1 year ago

My preferred way is not to overwrite the number on every input, but let the browser manage the field. Using /*@once*/ or untrack(value), you can set the value only once on instantiation, and then just read the value from the event (the browser should ensure you have the same value in your field as in your state).

This only works in the most simple examples and breaks when you add complexity:

katywings commented 1 year ago

Personally I also would love a standard approach for controlled number inputs handling these edgecases, as they also happened to me in the past.

In the meantime @mattbdc what you maybe will find useful is something like this: https://playground.solidjs.com/anonymous/88e0b341-340d-4f32-94f3-a9b21f7890ee

I wrote the numberInputHelper by looking at your description and at the react way :).

mattbdc commented 1 year ago

Personally I also would love a standard approach for controlled number inputs handling these edgecases, as they also happened to me in the past.

In the meantime @mattbdc what you maybe will find useful is something like this: https://playground.solidjs.com/anonymous/88e0b341-340d-4f32-94f3-a9b21f7890ee

I wrote the numberInputHelper by looking at your description and at the react way :).

This is interesting, it is a different approach than what I used, I annotated events with internal/external property via some event handler factories, and only reset value on last event being external, presuming that anything internal would be consistent. I need to look into it all again shortly and will digest the above approach.

atk commented 1 year ago

I really don't understand the obsession with controlled inputs, but if you must do it, you can use the @solid-primitives/selection primitive to fix the selection after overwriting the value.

mattbdc commented 1 year ago

I really don't understand the obsession with controlled inputs, but if you must do it, you can use the @solid-primitives/selection primitive to fix the selection after overwriting the value.

The issues go far beyond selection after overwriting the value. Jumping cursors are just the tip of the iceberg, you move past that in react days, its not a hurdle we are addressing/debating.. The edge cases of everything else are very real (and signifcant) - if you havent noticed them yet your end users certainly will..

ryansolid commented 1 year ago

Know what's odd? It didn't do this in 112. Although this issue still happens in 114. My first inkling is that this is a Firefox issue but Safari has the same behavior.

I tried the example in Preact. And while its behavior is identical in Chrome it does differ from us in new Firefox(and Safari), but it still has the problem of disappearing values (although it doesn't clear the input): https://preactjs.com/repl?code=aW1wb3J0IHugcmVuZGVyIH0gZnJvbSAncHJlYWN0JzsKaW1wb3J0IHsgdXNlU3RhdGUgfSBmcm9tICdwcmVhY3QvaG9va3MnOwoKZnVuY3Rpb24gQ291bnRlcigpIHsKCWNvbnN0IFt2YWx1ZSwgc2V0VmFsdWVdID0gdXNlU3RhdGUoIiIpOwoKCXJldHVybiAoCiAgICAgICAgPD4KICAgICAgICAgICAgPGlucHV0IHR5cGU9Im51bWJlciIgdmFsdWU9e3ZhbHVlfSBvbklucHV0PXsoZXZlbnQpID0%2BIHNldFZhbHVlKGV2ZW50LmN1cnJlbnRUYXJnZXQudmFsdWUpfSAvPgogICAgICAgICAgICA8ZGl2PkN1cnJlbnQgdmFsdWU6ICJ7dmFsdWV9IjwvZGl2PgogICAgICAgIDwvPgoJKTsKfQoKcmVuZGVyKDxDb3VudGVyIC8%2BLCBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgnYXBwJykpOwo%3D

By that I mean: (input) (output) Chrome/Edge 1 "1" 1. "1" 1.2 "1.2"

Firefox/Safari 1 "1" 1. "" 1.2 "1.2"

And I tried it in Svelte and it had the exact same behavior as well.

<script>
  let value = ""
</script>

<input type="number" value={value} on:input={(event) => value = event.currentTarget.value} />
<div>Current value: "{value}"</div>

And guess what? I tried in React and it too has the same behavior. This issue has nothing to do with controlled inputs. https://codesandbox.io/s/react-number-field-bug-ynxygx?file=/src/index.js

Ok. So Solid is the odd one out but I'd argue no one is handling this well. The fact that it doesn't clear the input seems odd in that it does clear the value. The reason for this behavior is they do a check against the current value of the input. And they see that both are "" and then don't update the field. But that is sort of a lie. Doing the the value check seems, not only slow (potential of causing reflows) but also just incorrect.

So the problem is we either want to lie to the input field, or we want a Signal that only holds a number. I feel the latter is better as you know your Signal might be used in more than one place.

So @Tuoris "workaround" is generally the right approach. The NaN check is basically deciding what the symbol you use for NaN is. In that case it is being set to "". However, if you used this in more than one place you would have needed to handle "" as well.

So this looks like a won't fix to me, but always interested to hear thoughts.

Tuoris commented 1 year ago

Hi, @ryansolid !

The reason this issue happens in Firefox 114 because they recently decided to match a specs: https://www.w3.org/TR/2011/WD-html5-20110525/common-microsyntaxes.html#valid-floating-point-number. Chrome do not follow the specs in this case, and Safari - does. I've reported this behaviour as bug to Firefox team (https://bugzilla.mozilla.org/show_bug.cgi?id=1836659) - but they won't fix it too.

For my personal project I will stick with current workaround, but someone will probably benefit from using text input instead of number input (as GOV.UK did - https://technology.blog.gov.uk/2020/02/24/why-the-gov-uk-design-system-team-changed-the-input-type-for-numbers/). And also, if there was a way to get a both number and raw text value for number input in vanilla JS - that would be great.