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

Add active prop to reduxForm #2856

Closed
wants to merge 3 commits into from
Closed

Add active prop to reduxForm #2856

wants to merge 3 commits into from

Conversation

vitbokisch
Copy link
Contributor

Adding new prop passing to form via reduxForm. The value shows up when any field is active, but it is not passing to form at all. The prop, for example, helps doing more ux stuff like showing form as a modal when user is interacting withing the form and so on. Hope this proposal is worthy for you as well.

Adding new prop passing to form via reduxForm. The value shows up when any field is active, but it is not passing to form at all. The prop, for example, helps doing more ux stuff like showing form as a modal when user is interacting withing the form and so on. Hope this proposal is worthy for you as well.
@codecov-io
Copy link

codecov-io commented Apr 26, 2017

Codecov Report

Merging #2856 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2856   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          67      67           
  Lines        1395    1396    +1     
======================================
+ Hits         1395    1396    +1
Impacted Files Coverage Δ
src/propTypes.js 100% <ø> (ø) ⬆️
src/reduxForm.js 100% <100%> (ø) ⬆️

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 48f1781...4566187. Read the comment docs.

Copy link
Collaborator

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but a prop saying the name of the active field would probably be more useful than this one.

@vitbokisch
Copy link
Contributor Author

You are right, I'm gonna update it

Copy link
Collaborator

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thoughts, @erikras?

@erikras
Copy link
Member

erikras commented May 4, 2017

Pro: definitely seems useful
Con: adds another reason for the entire form to rerender

When you tab from input 'foo' to input 'bar' form to another, does the active prop ever === undefined in its way between the two values?

@erikras erikras changed the title add anyActive prop to reduxForm Add active prop to reduxForm May 4, 2017
@erikras
Copy link
Member

erikras commented May 4, 2017

v5 had this and it was removed as a conscious decision in the rewrite of v6 to minimize form rerenders. I think this should be a selector. It would be trivial to write (use #2859 as a guide). I'd accept that PR. Going to close this one.

@lock
Copy link

lock bot commented Jun 1, 2018

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 Jun 1, 2018
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.

None yet

4 participants