Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

formatOnBlur overloading format/parse original meaning #560

Open
Andarist opened this issue Jul 11, 2019 · 8 comments
Open

formatOnBlur overloading format/parse original meaning #560

Andarist opened this issue Jul 11, 2019 · 8 comments

Comments

@Andarist
Copy link
Contributor

This API seems conceptually broken right now. With format & parse we have a nice parity:

  • format maps a state value into a displayable value (happens during render)
  • parse maps displayable value into a state value (happens in a callback)

formatOnBlur though prevents format from being called during render - which so far makes sense, but it calls format inside a callback. So the parity gets lost but this also makes parse weirdly unusable with this option - because it still also gets called inside a callback, so suddenly parse's and format's role is the same (?), they both write to state.

Not sure if the problem is recognizable from this vague description - if not please tell me so and I'll try explain it better.

@erikras
Copy link
Member

erikras commented Jul 12, 2019

I get it, and even more so, feel it. formatOnBlur is a bit of inelegance tacked on to a nicely balanced system.

To solve this, however, we need to come up with a solution to the very legitimate use case of allowing a user to type 24.3 and then, on blur, having the field adjust it to $24.30, just to give one use case. Any spreadsheet app will demonstrate that. Ideally the value would be stored as Number.

Ooooh, just typing that, an idea occurred to me. What if formatOnBlur was formatWhenInactive (inactive == does not have focus). This would allow the formatting to not interrupt the user's typing, and also keep it in render only. The only caveat would be that if your field is subscribed only to value and not to active, it might not feel the need to rerender when it is blurred. Thoughts? 🤔

@Andarist
Copy link
Contributor Author

I was also thinking about this a little before filing this issue. And putting aside the exact API which would enable this behavior, let's discuss the implementation stuff.

It seems like we agree that format should be render-only. This leads us to a natural conclusion that parse can be considered as "pre-commit" thing, whereby commit I mean putting a value into stored form values.

Having those semantics in place another conclusion I have is that parse shouldn't be run on unformatted value, because more often than not such value can be reliably parsed into a stored one. Consider keeping a Date in a form and allowing a manual edit on the linked field - we can't parse a text input back into a Date (to put it inside a form state) in the middle of editing.

The only idea I have right now to solve this is to store the unformatted value inside a component's state separately - sync it with form state by default, but when working in this "formatOnBlur" mode (which I believe is a highly useful feature) use that component's state value instead of the form value when editing and sync them only when we commit that value into a form (so on the blur and similar).

This would bring generally unwanted complexity into the implementation, but I don't know how to solve this better.

@tsufiev
Copy link

tsufiev commented Jul 15, 2019

Seems that I have recently stumbled upon the case of weird behavior of formatOnBlur parse. Let me explain my setup so it becomes clearer if it's the same issue or a different one.

So there is a text field in a form where byte values can be typed, in different forms: 1, 3434, 1 B, 10 K, 134MiB - are all valid byte values (there is a tricky regexp for parsing out byte suffix and allowing for an optional 'iB' which makes it look prettier). Naturally, I use parse prop for that field - to convert '<number> <byte suffix>' to a plain number. Then, there is a validate prop to check resulting number with isNaN() predicate - if it's actually NaN, that means that conversion failed and the value is incorrect indeed. And finally, there is format prop which converts number back to the byte representation, so if user typed 102400B, he would get 100.00 KiB in the input. Before I discovered formatOnBlur option, the formatter behavior was weird, because byte conversion messed with the user editing numbers or prefixes with Backspace, only selecting specific symbol and replacing it did work.

As soon as I reworked logic of the field, formatOnBlur started to shine. For that to happen I had to save current input's value in field's state, and pass it to the external world on onBlur event - effectively triggering parse callback, then validate fired which ensured that the value was ok. After that happened (meaning blur event) format put the parsed and validated value back into input, ensuring proper byte number and prefix are used.

Now about the weird issue. I've noticed that in presence formatOnBlur strange thing is happening on submit: numeric value is being formatted to byte representation and then is put into form state (see

if (formatOnBlur) {
const formatted = format(state.value, state.name)
if (formatted !== state.value) {
state.change(formatted)
}
}
), which is then validated. Obviously it doesn't work as expected, because my validate is expecting numbers, not strings.

So the question is: does it work as expected or is it a part of the problem you've been discussing in this issue? And second question, which is more important to me :), is how can I fix parse/validate/format behavior of my field in presense of formatOnBlur option when submit happens?

@Andarist
Copy link
Contributor Author

I believe it's the same issue, only has manifested differently.

@tsufiev
Copy link

tsufiev commented Jul 21, 2019

Since my description of the issue may be a bit vague, I edited Parse Format sandbox example to show the issue inside 1st field: https://codesandbox.io/embed/react-final-form-parse-and-format-v9r2i

My unmet expectations here are:

  1. Bytes value passed via initial values is not formatted to have byte suffix.
  2. On second blur event the bytes field passes formatted string into format, which doesn't work, because numeric value from parse is expected.
  3. The same thing as in (2) happens on submit.

@erikras what do you think about it?

@tsufiev
Copy link

tsufiev commented Jul 22, 2019

Just found the similar issue #383

@kneczaj
Copy link

kneczaj commented Oct 27, 2020

What you think guys about changing type of formatOnBlur from boolean to (value: FieldValue, name: string) => any like format prop is. This way it is possible to use both format and formatOnBlur simultaneously.

I have another use case for this:
I want to trim strings before sending values from input field to backend. I can use formatOnBlur to do this. However I want to use format (on keypress) in some of the these fields.

I can prepare PR for this. Regarding the compatibility with the current API formatOnBlur might be boolean | (value: FieldValue, name: string) => any and work as before when boolean

@k-sav
Copy link

k-sav commented May 26, 2022

To solve this, however, we need to come up with a solution to the very legitimate use case of allowing a user to type 24.3 and then, on blur, having the field adjust it to $24.30, just to give one use case.

Anyone have a good workaround for this use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants