cssinjs / theming

Unified CSSinJS theming solution for React
300 stars 39 forks source link

docs, tests and implementation review #2

Closed iamstarkov closed 7 years ago

iamstarkov commented 7 years ago

hey, @kof, @mxstbr, @nathanmarks, @kentcdodds.

I extracted theming from styled-components into separate package, wrote extensive documentation, covered every line and every case with tests. I also thought, that some solutions might need to use custom channel, so i added that as well.

So you might be interested in it

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling af0faaa080d263595b74cdb9399b19e7f22a6535 on dev-review into on master.

mxstbr commented 7 years ago

I don't think the withTheme HoC is enough for us, there needs to be some way of injecting the context stuff into an existing component. We don't want to wrap every styled-component in another component I don't think?

iamstarkov commented 7 years ago

@mxstbr thats the feature i wanted to, because i want the same in jss as well. Any suggestions on API design and name for this feature?

iamstarkov commented 7 years ago

@mxstbr what about current code, tests and docs?

iamstarkov commented 7 years ago

@mxstbr as far as this feature request we all want to make low level integration easier, i made another pull-request #3, please take a look.

iamstarkov commented 7 years ago

btw, @kof, @nathanmarks, @kentcdodds, i want you to review #3 as well =)

kentcdodds commented 7 years ago

cc @vesparny

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 268a88db8dd9e5ac7e8fa307bf1bee82afb9d609 on dev-review into on master.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 268a88db8dd9e5ac7e8fa307bf1bee82afb9d609 on dev-review into on master.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 0198e9a4223c3d9c387a0b2254ca5400da4c6f17 on dev-review into on master.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 1a3e02740cbd6ddcd5bb2f0ce4112204baf3974e on dev-review into on master.

iamstarkov commented 7 years ago

status update:

@kof, @mxstbr, @nathanmarks, @kentcdodds, @vesparny please review it again

regarding is-function and is-plain-object lets have separate discussions

if this is okay by you, let's merge this into master.

in the mean time i will work on #3

kof commented 7 years ago

LGTM

iamstarkov commented 7 years ago

@mxstbr, its your turn now =)

iamstarkov commented 7 years ago

—what is the time? —release time!

iamstarkov commented 7 years ago

good news @kof, @mxstbr, @philpl, @kentcdodds, @vesparny. y'all are added as collaborators in this project.

One thing about contributing: let's have pull-requests instead of pushing commits straight to the master

iamstarkov commented 7 years ago

theming@1.0.0 just has been published