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

[blur event] Why does handleBlur updates field value? #2768

Open
mereskin-zz opened this issue Apr 3, 2017 · 29 comments
Open

[blur event] Why does handleBlur updates field value? #2768

mereskin-zz opened this issue Apr 3, 2017 · 29 comments
Labels

Comments

@mereskin-zz
Copy link

mereskin-zz commented Apr 3, 2017

What is the current behavior?

onBlur and onFocus callbacks are required to mark field as touched to display per-field validation. onBlur and onFocus assume DOM event is passed and current field value corresponds to value of the underlying DOM element.

That behavior is not working for custom components. If custom component is used in place of input the value property of DOM element that originates event can not be equal to that of a component itself.

For example, we use CheckBoxGroup which has value property defined as arrayOf(string). This component still emits onFocus and onBlur events when individual checkboxes are checked/unchecked and passes DOM event through. This DOM event is used by blur, value is extracted from it and this value is no longer an array but string true, since this value is read from checked property of a checkbox.

What is the expected behavior?

onBlur event handler shouldn't update the value of the field and shouldn't assume that event object passed in is actually a DOM event. onBlur handler should update touched property of the field so that validation is still displayed.

Is there any reason why onBlur event updates form state? Can we use onChange events exclusively to update form state?

What's your environment?

All environments

@carlsverre
Copy link

I am also having an issue with Blur modifying the value of the field. The problem for me is that onBlur assumes that the event contains the current value - which is not true when you batch react updates and the form is updated via autocomplete. In this case the browser fires onChange and onBlur in the same frame and thus the onBlur will see the "last" value of the field since the HTML input hasn't actually had a chance to update.

Making onBlur not update the form value will resolve my problem.

@carlsverre
Copy link

I am currently getting around this issue by calling the onBlur handler with no event argument. This is supported by the tests and seems to do exactly what we want. You can see the test here:

https://github.com/erikras/redux-form/blob/master/src/__tests__/reducer.blur.spec.js#L70

@shodanuk
Copy link

shodanuk commented Apr 5, 2017

Another workaround is to use the parse (and / or possibly normalize) prop of Field to force the type value of that the blur action sets into state.

In my case, redux-form was converting number values of my fields into strings, causing issues with custom react-numeric input fields. Line 99 of ConnectedField calls onChangeValue which "normalises" the value of the field using a function passed into the parse or normalize prop of ConnectField. In the absence of these functions being specified, the value seems to default to a string.

For what's worth, this is all I passed into the parse prop:

<Field
  ...
  parse={(value) => (parseInt(value, 10))}
  ...
  /> 

For it's worth, I agree with OP that the value being changed on a blur event is an unexpected and unconventional side effect that should considered for removal unless there's a really good reason for it.

@mereskin-zz
Copy link
Author

mereskin-zz commented Apr 6, 2017

@carlsverre I tried your workaround: onBlur={() => { input.onBlur(); }} but it just clears the field in my case when I focus and blur.

@erikras any objections to removing the value from onBlur? I could make a PR.

@mereskin-zz
Copy link
Author

@shodanuk this is assuming that value is of another type but still correct. This may be true for fields that imitate input but this is not the case for complex fields, like CheckBoxGroup that I described above.

Such field consists of multiple checkboxes. onBlur is emitted with e where e.target is an individual checkbox that originated an event. There is no corresponding DOM element that exposes correct value (array of strings) that can be supplied with e.target.

@carlsverre
Copy link

As far as I can tell the fact that blur changes the value has been the case since the 0.7 release of the project (earliest reference I could find to setting an onBlur handler for any form component). You can see the code here:

https://github.com/erikras/redux-form/blob/e9b462cbb41bd8df3d27495c9911b9a2155d9b4e/src/reduxForm.js#L26

I don't see any issues or comments mentioning why the blur event does this in the entire history of the project. I briefly looked at almost every commit which touched any of the blur code.

Would really appreciate a comment from @erikras on why Blur changes the value.

@erikras
Copy link
Member

erikras commented Apr 9, 2017

Hmm... Blur changes the value to allow maximum flexibility in how the library is used.

Can we be guaranteed that every single input that calls onBlur also calls onChange on every change? No, we can't. I know for a fact that some people have written inputs to work with redux-form that, to save on Redux traffic – especially for complex structures, like Draft.js wysiwyg editors – keep the value in local state and only send the value to Redux onBlur.

However, I definitely see why someone might want to only flip the focus and touched flags onBlur. What if we allowed a second noUpdate parameter to onBlur, so that you could do:

<input {...props.input} onBlur={() => props.input.onBlur(undefined, true)}/>

Or optionally one could pass a noUpdateOnBlur flag to Field that would do this for you.

<Field name="firstName" component={renderInput} noUpdateOnBlur/>

I don't think that stripping the current value-setting functionality out of onBlur, like #2791 is doing, is the answer.

@mereskin-zz
Copy link
Author

mereskin-zz commented Apr 10, 2017

I see. I think it would be straightforward for those components to implement something like:

onBlur={() => {
  input.onChange('some new value');
  input.onBlur();
})

Anyway. Maybe we shouldn't go as far as having a separate flag for noUpdate and instead just fix when onBlur is called without an event argument.

Current behavior:
if onBlur is called without event argument field's value is set to undefined.

Desired behavior:
If onBlur is called without event argument, set touched to true and don't change the value.

What do you think @erikras ?

@carlsverre
Copy link

If current consumers of the library are depending on various forms of the existing onBlur behavior - it will be a lot safer/faster for us to add a new mode rather than modifying the existing behavior. In that case my vote is for supporting a second explicit parameter to the onBlur method. Possibly it should be an options parameter to make future upgrades non-breaking as well:

function onBlur(event, options) {
  const { noUpdateValue } = options;
  ...
}```

@joshbedo
Copy link

joshbedo commented May 1, 2017

Is there any fix for this right now? I'm using parse/format and any onblur event causes the field to be emptied.

@erikras
Copy link
Member

erikras commented May 4, 2017

Current behavior:
if onBlur is called without event argument field's value is set to undefined.

Desired behavior:
If onBlur is called without event argument, set touched to true and don't change the value.

@mereskin I could be misunderstanding you, but this is not true. Calling input.onBlur() with no parameters does not change the value. See: 6aec0c4

I must be misunderstanding the problem here. Can you re-explain? Or make a sandbox to demonstrate it?

@mereskin-zz
Copy link
Author

@erikras maybe that's only when parse/format is specified? #2768 (comment)

I'll try to make a sandbox tonight.

erikras pushed a commit that referenced this issue May 9, 2017
…ializing the form (#2835)

* passing the last initialValues into initialize action if it is reinitializing the form

* Added test related to #2809

* Removed meta.touched from FieldArray props (#2836)

* Add props to the params documented in the field-level validations example (#2855)

* Fix typo in docs/api/Field.md (#2864)

* Improve SubmissionError docs (#2873)

* Add a selector to get touched and visited props from state (#2859)

* added a selector (getFormMeta) which retrieves the form.{$name}.fields slice from state

* added documentation for getFormMeta selector

* Fixed field-level validation reset bug (#2888)

* deleteInWithCleanUp needs to check if there is a value in the state before deleting. (#2876)

* deleteInWithCleanUp needs to check if there is a value in the state before deleting, to avoid Invalid KeyPath errors

* Got rid of string compare

* clean script - added es folder to be rimrafed before build (#2860)

* Add a Field's initial value to props.meta.initial (#2858)

* Add meta.initial to Field props for access to a field’s initial value.

* es6 typo

* add meta.initial to the docs for Field

* Demonstrated that calling onBlur() does not change value #2768

* Better cleanup on UNREGISTER (#2889)

* Checkbox default behavior proposal (#2863)

instead true/false to be true/'' as the current true/false breaks pristine prop

* Fancy new npm5 lock file!

* Allow direct imports to reduce bundle size. (#2893)

* Fix deep equal on maps with different structures (#2892)

* Remove references to react-router from doc page. (#2898)

* Add PropTypes for the Field components (#2862)

* Add PropTypes for the Field components

* Export fieldInputPropTypes, fieldMetaPropTypes and fieldPropTypes

* Merged with changes from #2893

* Exports defaultShouldValidate and defaultShouldAsyncValidate (#2891)

* Exports defaultShouldValidate and defaultShouldAsyncValidate

These default functions are exported for external use - particularly so
that a user can wrap the default functionality with specialised behaviour.

To resolve #2890

* Export default shouldValidates in immutable wrapper

* Allow direct imports to reduce bundle size. (#2893)

* Fix deep equal on maps with different structures (#2892)

* Remove references to react-router from doc page. (#2898)

* Add PropTypes for the Field components (#2862)

* Add PropTypes for the Field components

* Export fieldInputPropTypes, fieldMetaPropTypes and fieldPropTypes

* Merged with changes from #2893
@dariospadoni
Copy link

I'm also struggling with the same problem, I prepared a sandbox here which shows the issue I have.

In my case I have a select control whose values is a Object which I stringify and parse back to JSON in the field. The value of the form is first the selected one but then (on blur event) it gets reverted to the original value. Am I doing anything wrong? Are there any workaround?

@ericbiewener
Copy link
Contributor

Looks like this issue is connected.

@dmason30
Copy link

dmason30 commented Jun 20, 2017

@erikras
I have a form that does some complex calculations onBlur that could dispatch a new value for the current field but after upgrading from 6.3.2 to 6.8.0 the onBlur is overwriting the calculated value with the raw value of the field after I have done the calculations.

I have created a sandbox below which shows a simple onBlur that rounds the number to the nearest 10 and you can see that the number remains not rounded because of the redux-form/CHANGE then redux-form/BLUR. I have put a setTimeout also in to show that it correctly rounds the number afterwards.

As a temporary fix I can put e.preventDefault() but to do this on every onBlur where we need to dispatch something seems unnecessary. I have also add an input doing this in the sandbox below.

Sandbox: https://codesandbox.io/s/l5XOknk05

@mlarcher
Copy link

Fwiw, it seems to me that a onBlur={e => { e.preventDefault(); } on the Field element brings us the expected behaviour with no downside. Gotta confirm there is no edge case though.

@martinbliss
Copy link

martinbliss commented Sep 3, 2017

@mlarcher is correct. Passing a NoOp function to onBlur on the Field node works. If you use custom field inputs often, you can actually do this at your custom component level by "extracting" the onBlur event handler that is passed as part of the input prop and substituting it for your own that calls e.preventDefault():

const CustomFieldInput = ({input: onBlur, ...inputProps}) => {
const handleBlur = e => e.preventDefault();
return <input onBlur={handleBlur} {...inputProps}/>; // note how `onBlur` from `Field` passed-in props is not being passed in here.
};

@An6r
Copy link

An6r commented Dec 8, 2017

@mlarcher Looks like the downside of using e.preventDefault() is clearly explained in the documentation

If you call event.preventDefault(), the BLUR action will NOT be dispatched, and the value and focus state will not be updated in the Redux store.

Probably focus state is not important in your case, but still...

@pvjones
Copy link

pvjones commented Dec 12, 2017

Using event.preventDefault() was not working for me, since input.touched was then not set to true (this is needed in my case for displaying input error hints). I think this is related to what @An6r mentioned above. It's not pretty, but I ended up passing down a callback that invokes input.onBlur without the event (this also satisfies my TypeScript compilation). If anyone has a better solution I'd love to hear it.

const renderChipInput: SFC<MergedProps> = ({
  input,
  meta: { error, touched },
  ...rest,
}) => {
  const value = input.value || List()

  const onRequestAdd = (chip: any): void => {
    if (invalidChip(chip)) { return }
    input.onChange(input.value.push(chip))
  }
  const onRequestDelete = (chip: any): void => {
    if (value.indexOf(chip) !== -1) {
      input.onChange(input.value.delete(index))
    }
  }

  return (
      <ChipInput
        value={value}
        onRequestAdd={onRequestAdd}
        onRequestDelete={onRequestDelete}
        helperText={touched && error}
        onBlur={e => input.onBlur(undefined)}
        {...rest}
      />
  )
}

const FormChipInput: SFC<BaseFieldProps<MergedComponentProps> & FormChipInputProps> =
  ({ name, componentProps, ...rest }) => {
    return (
      <Field
        name={String(name)}
        component={renderChipInput}
        props={componentProps}
        {...rest}
      />
    )
  }

@ghost
Copy link

ghost commented Apr 24, 2018

Same problem here, we are using react-select which has controled, non-input <div> under the hood and redux-forms's onBlur empties value of field. The problem is, that event's target is <div> so i does not contains value property. I have format/parse set on field and calling onBlur() without arguments empties field as well. Current workaround is override onBlur with noop, which does not set meta properties, but for my current logic, I don't need it.

@Yankovsky
Copy link

Any progress on this issue? I think it is a consumer's responsibility to call onChange whenever he wants. But I do understand that we don't want to introduce a breaking change. So what about this flag @erikras was talking about.

@Yankovsky
Copy link

Yankovsky commented Aug 8, 2018

For now I am just manually dispatching blur action with current value

handleBlur = () => {
	const {
		input: {
			name,
			value,
		},
		meta: {
			form,
			dispatch,
		},
	} = this.props

	dispatch(blur(form, name, value, true))
}

This is what happens when you call input.onBlur

And this is the blur action creator

@ladjzero
Copy link

ant-design's InputNumber hits this problem. onChange handler of InputNumber is fed with a number value, however onBlur is fed with a raw FocusEvent dispatched from the HTMLInputElement underneath.

onChange is called immediately before onBlur if the value is corrected by min-max validations. Maybe these two events occur in the same frame mentioned by @carlsverre , the value of InputNumber is always reverted by the raw value from FocusEvent unexpectedly.

@kaleem-elahi
Copy link

kaleem-elahi commented Nov 19, 2019

Another workaround is to use the parse (and / or possibly normalize) prop of Field to force the type value of that the blur action sets into state.

In my case, redux-form was converting number values of my fields into strings, causing issues with custom react-numeric input fields. Line 99 of ConnectedField calls onChangeValue which "normalises" the value of the field using a function passed into the parse or normalize prop of ConnectField. In the absence of these functions being specified, the value seems to default to a string.

For what's worth, this is all I passed into the parse prop:

<Field
  ...
  parse={(value) => (parseInt(value, 10))}
  ...
  /> 

For it's worth, I agree with OP that the value being changed on a blur event is an unexpected and unconventional side effect that should considered for removal unless there's a really good reason for it.

Cool! It works, But, for handling pristine also change input type to number.

@johnnybigoode-zz
Copy link

Using event.preventDefault() was not working for me, since input.touched was then not set to true (this is needed in my case for displaying input error hints). I think this is related to what @An6r mentioned above. It's not pretty, but I ended up passing down a callback that invokes input.onBlur without the event (this also satisfies my TypeScript compilation). If anyone has a better solution I'd love to hear it.

const renderChipInput: SFC<MergedProps> = ({
  input,
  meta: { error, touched },
  ...rest,
}) => {
  const value = input.value || List()

  const onRequestAdd = (chip: any): void => {
    if (invalidChip(chip)) { return }
    input.onChange(input.value.push(chip))
  }
  const onRequestDelete = (chip: any): void => {
    if (value.indexOf(chip) !== -1) {
      input.onChange(input.value.delete(index))
    }
  }

  return (
      <ChipInput
        value={value}
        onRequestAdd={onRequestAdd}
        onRequestDelete={onRequestDelete}
        helperText={touched && error}
        onBlur={e => input.onBlur(undefined)}
        {...rest}
      />
  )
}

const FormChipInput: SFC<BaseFieldProps<MergedComponentProps> & FormChipInputProps> =
  ({ name, componentProps, ...rest }) => {
    return (
      <Field
        name={String(name)}
        component={renderChipInput}
        props={componentProps}
        {...rest}
      />
    )
  }

I'm still having this issue.
´onBlur={e => input.onBlur(undefined)}´
Solved

"redux-form": "^7.0.4",

@yunusunver
Copy link

Using event.preventDefault() was not working for me, since input.touched was then not set to true (this is needed in my case for displaying input error hints). I think this is related to what @An6r mentioned above. It's not pretty, but I ended up passing down a callback that invokes input.onBlur without the event (this also satisfies my TypeScript compilation). If anyone has a better solution I'd love to hear it.

const renderChipInput: SFC<MergedProps> = ({
  input,
  meta: { error, touched },
  ...rest,
}) => {
  const value = input.value || List()

  const onRequestAdd = (chip: any): void => {
    if (invalidChip(chip)) { return }
    input.onChange(input.value.push(chip))
  }
  const onRequestDelete = (chip: any): void => {
    if (value.indexOf(chip) !== -1) {
      input.onChange(input.value.delete(index))
    }
  }

  return (
      <ChipInput
        value={value}
        onRequestAdd={onRequestAdd}
        onRequestDelete={onRequestDelete}
        helperText={touched && error}
        onBlur={e => input.onBlur(undefined)}
        {...rest}
      />
  )
}

const FormChipInput: SFC<BaseFieldProps<MergedComponentProps> & FormChipInputProps> =
  ({ name, componentProps, ...rest }) => {
    return (
      <Field
        name={String(name)}
        component={renderChipInput}
        props={componentProps}
        {...rest}
      />
    )
  }

Thanks, worked for me 👍

@helight59
Copy link

Using event.preventDefault() was not working for me, since input.touched was then not set to true (this is needed in my case for displaying input error hints). I think this is related to what @An6r mentioned above. It's not pretty, but I ended up passing down a callback that invokes input.onBlur without the event (this also satisfies my TypeScript compilation). If anyone has a better solution I'd love to hear it.

const renderChipInput: SFC<MergedProps> = ({
  input,
  meta: { error, touched },
  ...rest,
}) => {
  const value = input.value || List()

  const onRequestAdd = (chip: any): void => {
    if (invalidChip(chip)) { return }
    input.onChange(input.value.push(chip))
  }
  const onRequestDelete = (chip: any): void => {
    if (value.indexOf(chip) !== -1) {
      input.onChange(input.value.delete(index))
    }
  }

  return (
      <ChipInput
        value={value}
        onRequestAdd={onRequestAdd}
        onRequestDelete={onRequestDelete}
        helperText={touched && error}
        onBlur={e => input.onBlur(undefined)}
        {...rest}
      />
  )
}

const FormChipInput: SFC<BaseFieldProps<MergedComponentProps> & FormChipInputProps> =
  ({ name, componentProps, ...rest }) => {
    return (
      <Field
        name={String(name)}
        component={renderChipInput}
        props={componentProps}
        {...rest}
      />
    )
  }

I'm still having this issue.
´onBlur={e => input.onBlur(undefined)}´
Solved

"redux-form": "^7.0.4",

this action broke "asyncBlurFields"(

@helight59
Copy link

onBlur={() => {}}
this solution help me. Don't know why. AsyncValidate work fine

@R0kkusu
Copy link

R0kkusu commented Feb 18, 2022

I had the same issue with onblur and onfocusout, it change the input value to 'true' for some reason.
I worked around it by declairing a const that takes the current input value , and at the end of the code i set the input value to the previous const so it looks like nothing changed.
for example:
const p = document.createElement("p"); function validate_fullname() { const keepvalue = fullname.value; if ( fullname.value = "" || fullname.value.length < 4) { p.innerHTML = "fullname invalid"; fullname.after(p); fullname.value = keepvalue; }else { p.remove(); fullname.value = keepvalue; } }

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

No branches or pull requests