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: DropDownItem prop-types active prop (fix: #3598) #3608

Merged
merged 3 commits into from Apr 10, 2019

Conversation

@mxschmitt
Copy link
Member

mxschmitt commented Apr 8, 2019

Fixes #3598.

I hope it's okay, that I simplified these == null and != null statements. (active var type is null|bool and makeEventKey() is null|String)

Copy link
Member

taion left a comment

i don't think this is quite right... we want to treat false differently from null here

Copy link
Member

taion left a comment

this looks good

the one thing i might do naming-wise is – do active: propsActive in destructuring, then call the calculated value active

but that's super nit-picky

@taion
taion approved these changes Apr 8, 2019
@taion

This comment has been minimized.

Copy link
Member

taion commented Apr 8, 2019

should we disable the "automatically dismiss reviews" thing? assuming that was an automatic dismissal

@mxschmitt mxschmitt requested a review from jquense Apr 9, 2019
@mxschmitt mxschmitt merged commit cf13d90 into master Apr 10, 2019
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@mxschmitt mxschmitt deleted the bugfix/DropDownItem/prop-types branch Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.