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

fix: send an event-like object into Field onChange when no event (#4548) #4604

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dereksandkamp
Copy link

The docs mention that it is possible for an input to call input.onChange and pass either an event or a new value as the argument.

They also mention that it's possible to pass onChange to a <Field /> component, and that handler should receive an event as its first argument, with a preventDefault method on it which will prevent the change from being dispatched.

It's not explained in the docs that there's any connection between these two APIs. However, since this change, the consumer only receives an event object in Field.onChange if the underlying input has called input.onChange with an event.

Since many people (including myself) have been relying on the ability to BOTH pass a value to input.onChange and call event.preventDefault() within the Field.onChange handler, I think we should either make that possible or update the docs to explain the restriction. This PR makes it possible.

Closes #4548

@codecov-io
Copy link

codecov-io commented Feb 5, 2020

Codecov Report

Merging #4604 into master will not change coverage.
The diff coverage is 87.5%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4604   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          74       74           
  Lines        1745     1745           
=======================================
  Hits         1735     1735           
  Misses         10       10
Impacted Files Coverage Δ
src/ConnectedField.js 95.45% <87.5%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1c0b23...856130f. Read the comment docs.

@machinek56
Copy link

Hi guys! Any updates on this PR?
@dereksandkamp @iamandrewluca

@iamandrewluca iamandrewluca self-assigned this Mar 24, 2020
@iamandrewluca
Copy link
Member

I'll try to find some time this week to review this. From what I saw it seems this may be a breaking change.
Implicated in some urgent COVID-19 projects and don't have to much time to allocate for redux-form

@dereksandkamp
Copy link
Author

Thanks! For what is worth, from my perspective the breaking change already happened and this is a fixing change 😆

@machinek56
Copy link

@dereksandkamp, for now i wrapped your PR into own package as we extremely need it.

@duyenho
Copy link

duyenho commented Apr 27, 2020

👋 @iamandrewluca and @dereksandkamp
cc: @machinek56

Hope everyone is safe during COVID-19.
Would love to get this one in as well 🙂

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

Successfully merging this pull request may close these issues.

event.preventDefault is not a function in onChange(event, value, previousValue, name)
6 participants