-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Deprecate internal OutsideClickHandler in favor of react-outside-click-handler package #1191
Deprecate internal OutsideClickHandler in favor of react-outside-click-handler package #1191
Conversation
f5a01a3
to
9ac1bdd
Compare
|
||
// import { forbidExtraProps } from 'airbnb-prop-types'; // TODO: add to propTypes; semver-major | ||
import { addEventListener } from 'consolidated-events'; | ||
console.log(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should use console.warn
instead, i think
195703c
to
f4df2a8
Compare
…k-handler package
f4df2a8
to
5751221
Compare
So for some reason the status check is stuck even though the job ran successfully on Travis - https://travis-ci.org/airbnb/react-dates/builds/385911685 @ljharb is this safe to merge or is something broken? |
Refresh the page? Everything looks fine except that the coverage threshold still needs tweaking |
@ljharb when I click the travis link in coveralls (which what causes the pass or fail i thought), the travis job is green. Previously it told me that the branch coverage needed tweaking. Is it because of the missed lines in the diff? Incidentally this missed line is the console.warn in the OutsideClickHandler... so if that's the case, I'm inclined to just merge through. |
I think it's fine to merge through as long as it's not going to start failing on successive PRs. |
This is a patch because I don't actually delete the OutsideClickHandler in this change but rather ship it with a deprecation message.
to: @ljharb