-
Notifications
You must be signed in to change notification settings - Fork 48
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
Bugfixes: Unknown props, prop-types, inline-styles, and ESLint #18
Bugfixes: Unknown props, prop-types, inline-styles, and ESLint #18
Conversation
Note: Added yarn.lock file and note about contributing
onLoadFunction, | ||
resizeInterval, | ||
...other | ||
} = this.props; |
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.
The recommended way to _.omit
props before passing the rest down.
src/blur.jsx
Outdated
position: 'absolute', | ||
top: 0, | ||
left: 0, | ||
} : {}; |
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.
Added the enableStyles
because inlining them immediately lead to a breaking change on my end (since I wasn't using them).
I suppose this is substantial enough to increase the major version.
Totally amazing job @mherodev! Thank you! |
After putting together some relatively simple bugfixes, I ran into linting issues in VS Code that were best resolved by upgrading ESLint. And then upgrading dependencies. And reworking the eslint config.
So this PR is now far from a "logical chunk". I'm sorry.
Long story short, I replaced the existing eslint implementation with the airbnb configuration. I also went through and updated
Blur.jsx
to use the latest conventions and libraries (e.g.prop-types
).I also resolved issues #7 and #15 (which was my original intent, whoops).
Edit: And I included Prettier to passively enforce coding style.
Hopefully that's all well and good! Love the library.