Feedback

[kof]

1. class names based only +1

2. ui lib should ship without a theme +1

3. ui components should be requirable separately (we often just need the f..g button))))) +1

4. react-themeable +1

5. We need to distinguish between a theme and system styles. While changing colors, background images and rounded corners should not have much impact on transitions, changing sizes and margins will. There is a need for clear definition of what is a theme so that it doesn't breaks components.

6. How to pass the theme ... ? We can support 2 approaches at the same time.

   • A preferable approach - UI component wrapped into a theme component. This requires a user to wrap each component he is going to use. For large projects this overhead is not a problem, it allows also more project specific adjustments. For e.g. one could set a global default size for a button component within the wrapper.

     @useSheet(styles)
     export default class ThemedButton extends Component {
       static defaultProps = {
           width: ...,
           height: ...
       }
       render() {
         const {classes} = this.props.sheet
         return <Button theme={classes} {...this.props}  />
       }
     }
   

   In user land you import your themed button: import Button from '../ui/Button'

   • Optionally a theme over context can be supported by the UI lib. This is a dirtier but faster way. It can be used as a fallback if no theme prop has been passed. A ui lib name for e.g. "elemental" should be used as a convention for the namespace on the context object to avoid collisions.

       // This function can be part of react-theamable
       function getTheme(namespace, element) {
         let {theme} = element.props
         if (theme) return theme
         const options = element.context[namespace]
         if (options) theme = options.theme
         return theme
       }
   
   
       export default class Button extends Component {
         render() {
           const theme = getTheme('elemental', this)
           return (
             <div className={theme.button}>
             </div>
           )
         }
       }
   

7. I am against using static props/class props. They don't play nicely with higher order components.

[kof]

cc @markdalgleish

[NogsMPLS]

I've been working on an internal lib at work for awhile now and came to many similar conclusions.

Agree 100% on classnames as the base for styles. Just makes the most sense, as its the most generic/least specific, and should work well with solutions like react-themeable. The only problem I've been about to think of is that any change in class name is a breaking change, but thats obvious. I've been tinkering around with an idea to create a "deprecated soon" warning for css classnames being use on a page that will soon be changed (think react's console warning messages each time you upgrade react, but for css classnames).

React-themeable looks good also, though I've been have trouble wrapping my head using Jed Watson's ClassNames repo with it and without using JSX, but I'm sure that a failing on my part and not react-themeable.

Agree on self contained.

Shipping without a theme...I think the core lib should have as minimal styles as possible, but there are a few components that just need styling in them even at a base level. Think of things like Date Pickers and whatnot. If the lib is using a theme utility though, a must would be to have a default theme in that utility that ships with the lib. I think most people won't create their own theme from scratch, but rather build on top of/hack up whatever theme options are given to them by the lib author.

Oleg's number 5 is a good point. Which could be controlled with a theme component. Just restrict specifically what props/styles the theme component uses.

[nikgraf]

super invaluable feedback @kof @NogsMPLS

I like the idea of warning users for breaking changes in classnames. There could be utility function (only running in dev mode) which logs out errors in case the provided theme or global theme uses a certain function. Very much like React's dev experience.

@NogsMPLS I agree that a default theme should come with the library. I was thinking about dev have to make one import+function call to inject it. Thoughts?

I like the idea about number 5, but couldn't wrap my head around how this should work. When we shipped the initial version of Belle it came with a default theme + a Bootstrap theme. The same components were so different and hard to imagine what other themes might change or have in common. One experiment could be to create 4-5 themes and then see what they have in common.

[kof]

Regarding number 5: maybe a theme should consist of 2 parts: classes and options. Options contains all constants needed to configure a component to work well with given classes. In case of JSS, I would create a stylesheet which would use the constants to create the JSON. And those constants will be passed to the component as a part of the theme, so that they can be used for calculations.

[mxstbr]

I like that @kof, that sounds reasonable.

Provide a default theme, but make it easily overridable. Maybe have the default theme in the component itself, and make it customizable via props?

[plandem]

1. class names based only +1 There is quite good article about this subject: http://jamesknelson.com/why-you-shouldnt-style-with-javascript/

2. ui lib should ship without a theme +1

3. ui components should be requirable separately +1

4. We need to distinguish between a theme and core styles. + 1

4-6-7 - not sure right now.

[okonet]

Although I agree on all the points, I think that consumers (i.e. other UI developers) do not really care if some UI lib uses classes or inline styles underneath as long as you can override the look and feel of it easily. The good news is projects like https://github.com/markdalgleish/react-themeable or https://github.com/andreypopp/rethemeable allow using any available method of styling components. I think this is really important and should not be discouraged because of personal preferences. I'm not a fan of inline styles myself, but saying all libs should follow one approach will alienate it for some developers.

I really like the approach of VicotryJS in describing how to write components using its architecture: https://github.com/FormidableLabs/builder-victory-component/blob/master/dev/CONTRIBUTING.md#component-style

To me, the ultimate goal of such project would be an API description + a set of tools (docgen, linters, etc) to allow other UI-lib authors integrate into their existing projects with ease. It seems there are too many different UI-libs for React being developed ATM and it's almost impossible to combine or exchange them without rewriting lots of own code.

Regarding themes: I think the most idiomatic way to do would context. But I have too few experience with any of these techniques. So, I'd rather leave the decision to smart people :)

[plandem]

also it's good idea to separate validation. E.g. at my current project i'm using: https://github.com/davidkpiano/react-redux-form and quite happy with it.

[nikgraf]

After letting your feedback sink in for a little while in addition to talking to some people I figured this Factory Pattern approach might do the job.

@kof the factory function to create a theme element also contains defaultProps next to theme which allows us to set sensible defaults.

@okonet love you idea with the linters. this we we could ensure certain quality & a good style.

Regarding point 5 (theme vs core style). I think this then could be an implementation detail of a styled UI kit package which the core (unstyled) components don't have any related code.

I'm curious why you think :smile:

[nikgraf]

@andreypopp I wonder what you think about it …

[mxstbr]

@nikgraf :+1: I love that, especially with CSS Modules, i.e.

const customTheme = {
  questionMark: styles.questionMark,
  visibleContent: styles.visibleContent,
};

[nikgraf]

@mxstbr in this case you don't even have to do the mapping. You can directly pass the styles into the theme. Checkout https://github.com/nikgraf/future-react-ui/blob/master/app/components/HintCssModulesExample/index.js

[andreypopp]

For our internal UI component library we use an approach described as React Stylesheet.

Actually this conceptually similar to factory pattern described by @nikgraf but with more convention around how factory is implemented and how mechanism for theme overrides works.

The idea is that a component which is meant to be styled in a few different ways define a stylesheet as a mapping from names to React components, for example:

class Button extends React.Component {

  static stylesheet = {
    Root: 'button',
    Caption: 'div',
  }

  render() {
    let {caption} = this.props
    let {Root, Icon} = this.constructor.stylesheet
    return (
      <Root>
        <Caption>{caption}</Caption>
      </Root>
    )
  }
}

So that render() is defined not in terms of concrete DOM components but in terms of components from a stylesheet.

This way we are agnostic of what underlying mechanism is used for styling DOM elements.

Then React Stylesheet defines a function which allows to derive a styled component from an original one.

For example if we want to style out <Button /> component with classes from some global stylesheet:

import {style} from 'react-stylesheet'

let SuccessButton = style(Button, {
  Root(props) {
    return <button {...props} className="ui-Button--success" />
  },
  Caption(props) {
    return <div {...props} className="ui-Button__caption--success" />
  }
})

This works with CSS Modules and inline styles as well, with JSS or good old SASS.

Nested overrides

The mechanics is a little more sophisticated though, the style(Component, stylesheetOverride) applies in a nested fashion so you can restyle components deep inside easily.

What if you have a <Dialog /> component and it has a button:

class Dialog extends React.Component {

  static stylesheet = {
    Root: 'div',
    Button: Button, // defined above
  }

  render() {
    let {Root, Button} = this.constructor.stylesheet
    return <Root>{children}<Button /></Root>
  }
}

Now style() can be used to produce a new styled dialog variant <Dialog /> along with a new styled <Button /> inside:

let StyledDialog = style(Dialog, {
  Button: {
    Root: (props) => <button {...props} style={{background: 'red'}} />
  }
})

Now <StyledDialog /> has a button inside with red background.

Granular styling API

Components can define a granular interface for styling themselves by defining static style(override) method. That way you can control how and what exactly could be overridden:

class Button extends React.Component {
  static stylesheet = {
    Root: 'button',
    Caption: 'div',
    Icon: null
  }

  static style({icon, ...override}) {
    // allow easy icon override
    return style(this, {
      Icon: (props) => <span {...props} className={`glyphicon-${icon}`} />,
      ...override
    })
  }

  render() {
    let {Root, Icon, Caption} = this.props
    return ...
  }
}

Then style function will call into this static method when override happens:

let ButtonWithMakeIcon = style(Button, {icon: 'plus'})

[andreypopp]

Regarding propagating theme through the React's context mechanism.

Name-clashing. It can be solved by using not string keys but Symbol keys as theme keys:

class Button extends React.Component {
  static themeKey = Symbol('Button.themeKey')
  render() {
    // resolve theme using `this.constructor.themeKey` from context.
  }
}

Then

let Theme = {
  [Button.themeKey]: { 
    // theme for Button
  }
}

That's how Rethemeable lib works.

But we found that we usually don't want to use context for theming as we don't have such requirements as switching themes in an app via some sort of theme switcher. Instead we use React Stylesheet to predefine themed components for some particular app and then use them directly.

[andreypopp]

As for DOM styling mechanism we use a CSS in JS approach.

React Stylesheet allows to configure fallback for DOM components so we style DOM components directly when styling composite components. The example with <Button /> above would look like:

import {style} from 'react-stylesheet'
import {style as styleDOM} from 'react-don-stylesheet'

let SuccessButton = style(Button, {
  Root: { // this calls into `styleDOM` actually as `Root` is defined as `button` DOM component
    backgroundColor: 'green',
    ...
  }
}, {styleDOM: styleDOM})

[NogsMPLS]

The unstyled lib + theme ui kit idea is interesting, because it seems like the consensus would be that we end up with 3 "types" of components.

I'm going to take some terminology for this article: https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.pbtswjdvc:

We would have a Presentational/Dumb Component for markup, a Styled Component that sets base styles and accepts themeing props, and a Container/Smart component in an actual App to handle state and business logic.

Which is interesting, because now it starts to look very much like the old school 'separation of concerns' pattern where we keep markup, styles and logic all separated, but still all inside React.

[kof]

+1 on using Symbol when using context ( @andreypopp ) +1 component factory ( @nikgraf )

At the end, both approaches mean that receiver component needs to take a theme and theme options via props or context.

We can provide a function implemented by react-themeable which will pick the theme from props first, if not found from context. This way component creators don't need to care about how to get the theme.

A theme for one component should bring a map of classes and options object. This will allow to have static cachable styles as well as options to configure component to match those styles.

[kof]

The need for both context and props theme passing I can see: flexibility to change a theme for a specific component at any node in react's render tree.

• Use context based theme passing if you want just one theme
• Use props if you want different themes everywhere (who knows why)
• Use context as a base theme passing mechanism, but props for specific variations in edge cases.

[nikgraf]

I'm a bit worried/scared of introducing context for theming. If we solely rely on props then components theming has this functional character: "calling a function f twice with the same value for an argument x will produce the same result f(x) each time". Context behave like side-effects influencing the styling and also a bit unpredictable from where things are coming. In addition this opens up the question if we merge the theme or overwrite the whole thing.

In addition I wonder how much value it has to theme a whole section differently. Applying a few theme props might do the job as well or do I miss real world use-cases (that I haven't experienced yet)

Thoughts?

[ustccjw]

How about use custom-tag and structure of the selector to define style:

• Component use custom-tag as the root, each component has a unique tag.
• Using custom-tag as css-wrap to ensure localization in a component.
• Do not use class attribute in inner component.

Component is the basic semantic unit, we do not care the internal implementation, so we do not need class attribute in inner component. Using structure of the selector can make html structure first. If you can not define your style clearly, maybe you need to split some children components.

[nikgraf]

@ustccjw can you provide an example? (would help a lot)

[kof]

@nikgraf regarding context, I understand the fear that its possible to redefine theme at any point via context and it might be hard to find out where. For this case we can have a restriction that a theme over context for the same components set, can be defined only once. Once its defined, overwrites over context are not allowed, do it via props. So you can be sure where to find it.

I think the Idea for using context is all about to provide a great UX from the beginning with a minimal setup.

[sergey-lapin]

I have a feeling that contexts suits as well for theming as it suites great for localization and I just don't get a fear that a lot of people have about them. Common misconception that contexts are implicit. Context is as explicit as props, maybe even more - if you don't declare smtn in contextTypes, you won't get it. Props will be passed even if you don't declare them in propsTypes. Problem with the context that it is just another mechanism to pass data and a lot of people just don't know about it or don't know how it works and as the result smth unexpected for them happens. @nikgraf As for functional greatness of props passing approach - you always can wrap your clean functional components in some dirty context based stuff, thats how redux connect works for example.

[andreypopp]

@lapanoid the question is what context gives you for theming what cannot be implementing using props? I see context being useful only for UI with theme switcher where user can dynamically change theme in application runtime (this is needed in i18n and this is why context for i18n is probably an ok idea). Usually you don't see such theme switcher in apps though and UI is assembled through pre-made themed components.

[kof]

@andreypopp simplicity of the setup. You can wrap an entire application into one Theme component.

[andreypopp]

@kof I feel like this is only theoretically simpler. In practice you'd need to worry you have enough things configurable/styled via context vs. props.

[sergey-lapin]

@andreypopp Basically to theme whatever Component I want in any position in the tree.

<Root>
  <Component1>
    <Component2/>
  </Component1>
</Root>

I need to pass props here through Component1 to reach Component2, with context approach Component1 is totally unaware of this. Problem is getting much worse with project and team scale.

[ustccjw]

@nikgraf

export default class ArticleList extends React.Component {
    render() {
        const { articles } = this.props
        const articleComponents = articles.map(article =>
            <Card key={article.number} article={article} />
        )
        return (
            <ideal-articlelist>
                {articleComponents}
            </ideal-articlelist>
        )
    }
}

ideal-articlelist {
    display: block;
    ideal-card {
        max-width: 800px;
        margin: 40px auto;
    }
}

export default class Card extends React.Component {
    render() {
        const { article } = this.props
        return (
            <ideal-card>
                <Markdown content={article.introduction} />
                <Link to={`/articles/${article.number}/`}>View More</Link>
            </ideal-card>
        )
    }
}

More info can check: https://github.com/ustccjw/tech-blog/blob/master/client%2Fcomponent%2Farticle-list%2Findex.jsx

[andreypopp]

@lapanoid my assumption is that you don't need to theme a component tree in an app but build one using themed components.

[kof]

@andreypopp I would probably do that too, however building simple things would feel like big overhead ...