Skip to content
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

Document namespaced theme feature #31

Open
javivelasco opened this issue Nov 4, 2016 · 7 comments
Open

Document namespaced theme feature #31

javivelasco opened this issue Nov 4, 2016 · 7 comments

Comments

@javivelasco
Copy link
Owner

Now it's possible to namespace a theme by using a themeNamespace property for the component. This avoids collisions between modules of different components using the same classname. It's a feature that it's working and it's published but not documented. We need to write a section detailing the API which is very simple!

@raveclassic
Copy link
Contributor

I think it would also be nice to update the rest of the docs including new features: nested themes, theme spreads etc.

@artem-popov
Copy link

Do you plan to support namespaces in default/from context theme?

@raveclassic
Copy link
Contributor

raveclassic commented Jun 16, 2017

@artem-popov The problem with namespaces is that themr relies on the fact that classname string starts with namespace and checks it with startsWith. But actual classname construction is controlled by css-loader and it may prepend something to the classname with some different configuration.
Therefore I would suggest using nested themes - if your parent component contains some theme for its child component like here:

//parent.css
.container {
 ...
}

.child__container {
}
.child__content {
}

then instead of using themeNamespace as child__ you should move child's styles to a separate module:

//parent.css
.container {
}

//parent-child.css
.container {
}
.content {
}

Then when rendering you should pass child's theme object instead of namespace string.
Instead of

import { themr } from 'react-css-themr';
import * as css from './parent.css';
const Parent = ({theme}) => (
  <div className={theme.container}>
    <Child theme={theme} themeNamespace="child__"/>
  </div>
);
const ThemedParent = themr(Symbol(), css)(Parent);

you should use

import { themr, themeable } from 'react-css-themr';
import * as css from './parent.css';
import * as childCss from './parent-child.css';
const Parent = ({theme}) => (
  <div className={theme.container}>
    <Child theme={theme.Child}/>
  </div>
);
const theme = themeable(css, {
  Child: childCss
});
const ThemedParent = themr(Symbol(), theme)(Parent);

Please correct me if I'm wrong because I don't fully understand the purpose of theme namespaces.

UPDATE: nested themes are supported on every level of theme merging: context, props and default configuration.

@artem-popov
Copy link

Sorry for late answer.
Let me explain my case.

I''m very often use HOC's and each of the HOC's add some feature to component and possible has it's own styles.

Styles are already in different files, and in my HOC I'm adding default theme.

I'm adding namespace with sass:

.namespace {
   &someClass { ... }
}

So, I have all themes merged all together:

  • base one in context
  • passed theme from prop
  • default theme added by HOC
  • default theme of the component

In result I want to put component default theme and HOC's theme in different namespaces to not intersect names and do not worry about of this.

@raveclassic
Copy link
Contributor

raveclassic commented Jun 22, 2017

I don't see any problem with HOC's: you can still split HOC's styles and its child styles into different files and combine them into one theme object using themeable and nesting.
You gain all the profits you've enumerated and you don't need to mess with string values.

Please provide more concrete example if I don't get you right. But still I think namespaces in actual theme values instead of theme keys aren't a good solution because the values are details controlled by lower level of abstraction - webpack-loader. Even more, default configuration of latest version uses plain hashes in runtime instead of readable classnames.

UPDATE:
I checked the code once again and I'm wrong with values. Namespaces are applied to theme keys.
To sum up - I think that plans about supporting merging of namespaced themes on all levels should be considered by @javivelasco because I don't actually use them.

PS. You could still open a PR :)

@raveclassic
Copy link
Contributor

@artem-popov
What's more many devs use react and themr with typescript and namespaces break possibility to check the type of theme received in props because actual keys differ from defined in props interface. So I think it's another point for nested themes.

@artem-popov
Copy link

I'll think about that :)

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

No branches or pull requests

3 participants