-
Notifications
You must be signed in to change notification settings - Fork 16
Allow use of react inline styles on root component #7
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
base: master
Are you sure you want to change the base?
Conversation
… style prop - previously committed test now passes. Bump minor version number to indicate new, backwards-compatible feature.
|
Just to mention as well, on my fork I also have a branch 1.x, building on this, which upgrades to minimum peer dependency of react (and new react-dom package) 0.14.x It doesn't add any functionality, but just gets everything lined up with react 0.14 (some tests fail due to slightly different markup, there are some warnings and deprecations which had to be cleaned up) and uses the stateless functional component feature of 0.14 which promises performance optimisations in future versions. If you want I'll create a separate PR for this, after this one. Obviously it's a major version bump and breaking (dependency) change so, I'll do it however you'd prefer :-) |
package.json
Outdated
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.
please leave a previous version for now - it's better to commit it separately
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.
Sorry this is actually just a minor version bump on 0.x for the new props feature.
I was thinking the react 0.14 additions would justify a major version bump to 1.x, so that's in that branch - not clear what semver dictates about breaking peer dependencies, if you want to keep it as another minor bump I'll do that instead.
|
Thank you for this! I don't have much time to deal with it right now, but I'll try to do it later (this lib needs some updates, including renaming). PR for 0.14 wouldn't hurt also :) |
…y extra provided props through to the root element, instead of only supporting specified ones.
|
No problem, have made the suggested changes but left the versioning as-was as I think a minor then a major bump, respectively for the two PRs, follows semver. Have made the 0.14 PR :-) |
|
I just don't think PR is a good place for versioning. What if I already bumped a version? Also, as far as I know, breaking change is when first non-zero version number changes, not just major version. So if major version is zero, |
|
Good point - I'll roll the versions back in both PRs. Re major and minor version numbers, I was following the definition from here Just as an aside, react itself does it sort of in the same way you describe, except they technically don't make breaking changes between subsequent minor bumps, just deprecations in the last minor version api - so it's a bit of a grey area. Anyway I'll roll the versioning back, it's your call :-) |
|
Yeah, I saw that, it just says "Major version zero (0.y.z) is for initial development. Anything may change at any time", so it's like anything might be breaking. But I like that "0.breaking.feature" / "breaking.feature.fix" scheme, it feels reasonable enough for me. Thank you for cooperation! :) |
|
@davnicwil we got a merge conflicts, sorry! Can you please rebase? |
Adds a feature to pass a style prop to the root component, which is then applied as an inline style to the root html element.
Much the same idea as the className prop adding a class to the root html element, but for those who prefer using inline react styles to stylesheets.
Unit tests included, and for what it's worth have manually tested integration in a project - all works fine!