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 type of Redux Form event handlers. #31607

Merged

Conversation

davidgomes
Copy link
Contributor

@davidgomes davidgomes commented Dec 22, 2018

This PR updates the event handlers for the onBlur, onDragStart, onChange, onDrop and onFocus to support the optional name (field name) parameter.

I ran npm run test and npm run lint redux-form. Both returned no errors.

https://redux-form.com/7.4.2/docs/api/field.md/#-code-onblur-event-newvalue-previousvalue-name-gt-void-code-optional-

Here above is the API definition for the functions that I changed, which were missing the last argument.

@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels Dec 22, 2018
@typescript-bot
Copy link
Contributor

typescript-bot commented Dec 22, 2018

@davidgomes Thank you for submitting this PR!

🔔 @CarsonF @aikoven @LKay @bancek @alsiola @tehbi4 @huwmartin @ethanresnick @Reggino @maddijoyce @smifun - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Dec 27, 2018
@typescript-bot
Copy link
Contributor

After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience!

Copy link
Contributor

@mshaaban088 mshaaban088 left a comment

Choose a reason for hiding this comment

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

@davidgomes Thanks for fixing type definitions.
I appreciate if you could take a look at my suggestions

types/redux-form/lib/Field.d.ts Outdated Show resolved Hide resolved
types/redux-form/lib/Field.d.ts Outdated Show resolved Hide resolved
@davidgomes
Copy link
Contributor Author

@mshaaban088 great suggestions, I edited the PR's description to account for these changes.

@mshaaban088
Copy link
Contributor

@davidgomes Awesome!
One minor suggestion, as onDrop is implicitly affected, it makes sense to add it too to the description ;)

@davidgomes
Copy link
Contributor Author

@davidgomes Awesome!
One minor suggestion, as onDrop is implicitly affected, it makes sense to add it too to the description ;)

Done!

@mshaaban088
Copy link
Contributor

Thanks @davidgomes

@typescript-bot typescript-bot added Other Approved This PR was reviewed and signed-off by a community member. and removed Unmerged The author did not merge the PR when it was ready. labels Dec 31, 2018
@sheetalkamat sheetalkamat merged commit 6b9f1df into DefinitelyTyped:master Dec 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Other Approved This PR was reviewed and signed-off by a community member. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants