Skip to content

AutoKey feature #5

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

RafalFilipek
Copy link

While key is only required by Radium it would be nice to use theme function without it.
I wrote simple wrapper for theme function to address this feature. So:

import themeable, { autokey } from '../src';
const theme = autokey(themeable(classes))
theme('foo', 'bar')

Thank you for your work!

@NogsMPLS NogsMPLS mentioned this pull request Jan 25, 2016
@vyorkin
Copy link

vyorkin commented Feb 5, 2016

👍 want this

@markdalgleish
Copy link
Owner

Sorry I didn't see this earlier.

The main question I have is, if you don't support Radium, doesn't this defeat some of the purpose of this library? Part of what I'd like to achieve is that if your open source component says it uses react-themeable, people know that it will work for them even if they're using Radium.

@alexkuz
Copy link

alexkuz commented Feb 7, 2016

By the way, why it should manage keyprop at all? As for me, the fact that Radium needs it doesn't really mean it relates to theme managing. As long as this lib does nothing with it (except passing it to output object), isn't it better to just warn users they should always provide a key to component by themselves?

@markdalgleish
Copy link
Owner

I chose to enforce that component authors provide a key for each element so Radium support can be guaranteed, without requiring inspection of rendered elements after the fact which is much more heavy handed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants