Skip to content

Conversation

@tnrich
Copy link
Contributor

@tnrich tnrich commented Jul 19, 2019

Closes #3946

Co-Authored-By: Andrew Luca <thendrluca@gmail.com>
@codecov
Copy link

codecov bot commented Jul 29, 2019

Codecov Report

Merging #4507 into master will decrease coverage by 1.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4507      +/-   ##
==========================================
- Coverage     100%   98.96%   -1.04%     
==========================================
  Files          74       74              
  Lines        1744     1743       -1     
==========================================
- Hits         1744     1725      -19     
- Misses          0       18      +18
Impacted Files Coverage Δ
src/createReducer.js 100% <100%> (ø) ⬆️
src/createReduxForm.js 93.88% <0%> (-6.12%) ⬇️
src/handleSubmit.js 98.48% <0%> (-1.52%) ⬇️

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 d7d4e0b...4063e50. Read the comment docs.

@tnrich
Copy link
Contributor Author

tnrich commented Jul 29, 2019

@iamandrewluca I tried to make the changes but I'm not sure if I got them all. Could you let me know if I need to make anymore?

@tnrich
Copy link
Contributor Author

tnrich commented Jul 29, 2019

Also, @iamandrewluca it looks like the tests are failing for an unrelated error:

    console.error node_modules/jsdom/lib/jsdom/virtual-console.js:29
      Error: Uncaught [Error: Fields must be inside a component decorated with reduxForm()]

Maybe I need to merge in the latest commits on master?

@unrevised6419
Copy link
Member

@tnrich those are just console.error from code, they are expected. I think we should run tests in silent mode to have a more cleaner output.

tnrich and others added 2 commits July 29, 2019 16:29
Co-Authored-By: Andrew Luca <thendrluca@gmail.com>
Co-Authored-By: Andrew Luca <thendrluca@gmail.com>
@tnrich
Copy link
Contributor Author

tnrich commented Jul 29, 2019

Okay awesome I applied your suggestions. Thanks! Is it all ready to go now?

@unrevised6419
Copy link
Member

@tnrich yeap, we just need to wait for an project collaborator :)

@tnrich
Copy link
Contributor Author

tnrich commented Jul 30, 2019

Perfect thanks so much! I hope it'll go through!

@unrevised6419
Copy link
Member

unrevised6419 commented Jul 30, 2019

Would be great to have title more descriptive, may catch collaborators eyes

fix(reducer/change): allow `change` to handle `undefined` payload

and include in body

Closes #3946

https://help.github.com/en/articles/closing-issues-using-keywords

@unrevised6419
Copy link
Member

Also I would opt to deprecate using this initial === undefined && payload === '' in change.
This allows only to clean field when initial value is undefined.
User should take care to what value is changing.

@unrevised6419 unrevised6419 self-requested a review July 30, 2019 13:03
@renatoagds renatoagds added this to the v8.2.6 milestone Jul 30, 2019
@tnrich tnrich changed the title fix #3946 fix(reducer/change): allow change to handle undefined payload Jul 31, 2019
@tnrich
Copy link
Contributor Author

tnrich commented Jul 31, 2019

Also I would opt to deprecate using this initial === undefined && payload === '' in change.
This allows only to clean field when initial value is undefined.
User should take care to what value is changing.

How should I go about doing that? @iamandrewluca

@unrevised6419
Copy link
Member

Wi'll think of that in a future refactor.

@unrevised6419 unrevised6419 merged commit 04171fb into redux-form:master Jul 31, 2019
@renatoagds
Copy link
Contributor

Published in v8.2.6.

@renatoagds renatoagds mentioned this pull request Aug 13, 2019
@lock
Copy link

lock bot commented Sep 12, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calling Field's input.onChange(undefined) does not set field value to undefined.

3 participants