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

Passing local classes to unknown children #23

Closed
oliverbenns opened this Issue Jul 29, 2015 · 15 comments

Comments

Projects
None yet
8 participants
@oliverbenns

oliverbenns commented Jul 29, 2015

E.g.

Knowing the children

Like my comment here. This is great, we know the children, and can specify the button class and be done with it.

render() {
    return (
        <div className={ classes }>
            <h1 className={ styles.text }>{ this.props.label }</h1>
            <Button href={ this.props.button.href } className={ styles.button }>{ this.props.button.label }</Button>
        </div>
    );
}

Not knowing the children

  • Children could be a button, but it could also be a dropdown etc ( styles.dropdown , styles.button )
  • They're not really 'states' of these components, as it's particular to the parent. E.g. adjusting some layout particular that only happens in this parent component.

So how could I go about solving this?

render() {
    return (
        <div className={ classes }>
            <h1 className={ styles.text }>{ this.props.label }</h1>
            { this.props.children }
        </div>
    );
}
@ojame

This comment has been minimized.

ojame commented Aug 4, 2015

When you're passing the components as children, eg:

<Parent>
   <Button />
  <Dropdown />
</Parent>

You should know the context that they're in. You know just as much information about the children in this component, as you do in the Parent component itself. This is where your className should be defined:

<Parent>
   <Button className={styles.buttonWithContextOfParent} />
  <Dropdown className={styles.dropdownWithContextOfParent} />
</Parent>

If it's because you're trying to apply classNames to whatever children are supplied, under all circumstances and with total authority (leaving all of the concern to the parent, which, is probably a bad idea) you could:

cx({
   [styles.button]: child.type.displayName === 'Button',
   [styles.dropdown]: child.type.displayName === 'Dropdown',
})
@serapath

This comment has been minimized.

serapath commented Aug 4, 2015

Hey, I'm not so sure i exactly grok what this is about, but I think I have a similar problem and just wanted to tell how I'm trying to solve it.

I'm not using css-modules yet and I'm not using jsx yet.

I'm using atomify, thus the html & js is handled by browserify and the css is handled by reworkcss where @imports work like require().

The way I try to solve what you seem to talk about is:

  • For each child component the parent has a child dom container controlled by the parent
  • For each child component the parent can pass params to the child
  • For each child the parent can pass styling variables to the child
  • If the parent does not pass anything to the child, the child will use default values
    The current approach might be a bit verbose and requires some manual discipline and also has some limitations, so i would like to switch to css-modules soon.

So what I currently do is:

COMPONENT - HTML & JS

Menu Component

<!-- Menu.html -->
<div class="Menu">
  <div class="Menu_title"></div>
  <div class="Menu__logo----Logo"></div>
  <div class="Menu__navigation---Navigation"></div>
</div>
var Logo = require('logo')  // default logo component
var Navigation = require('navigation') // default navigation component

var template = require('./Menu.html')
var __ = document.createElement('div')

module.exports = function Menu (dom, data, children) {
  // DOM
  var CONTAINER    = dom
  var COMPONENT    = (__.innerHTML=template,__.childNodes[0])
  var __title      = COMPONENT.querySelectorAll('.Menu__title')[0]
  var __logo       = COMPONENT.querySelectorAll('.Menu__logo----Logo')[0]
  var __navigation = COMPONENT.querySelectorAll('.Menu__navigation')[0]

  // DATA
  var titleData = data.title || 'Hello World'
  var logoData = data.logo || {} // default "Logo Component" doesn't use data
  var navigationData = data.navigation || {} // default "Navigation Component" takes JSON menu tree

  // CHILDREN
  var Logo__ = children.Logo || Logo
  var Navigation__ = children.Navigation || Navigation

  // INIT
  // Menu Component
  __title.innerHTML = titleData
  // Children Components
  var API_logo = Logo__(__logo, logoData /*, no children */)
  var API_navigation = Navigation__(__navigation, navigationData, {/*none -> nav uses default*/})

  // RENDER & RETURN API
  CONTAINER.appendChild(COMPONENT)
  var API = {/*
    add methods to interact with menu from the console
    feel free to expose 'API_logo' and/or 'API_navigation'
  */}
  return API
}

Usage (of Menu Component)

var menu = require('menu')
var data = {
  title: 'My Dummy Page',
  logo: './logo.jpg',
  navigation: {
    home: '#home',
    about: '#about',
    /* ... */
  }
}
var children = undefined // use default menu children components
menu(document.body, data, children)

COMPONENT - CSS

The styling is done in parallel using the css files for each component.
Basically, each component specifies a lot of variables which can be "passed in" by the parent component, but if the parent component doesn't do so, it falls back to the default values

Logo Component

:root {
  --Logo_width: 2px;
  --Logo_color: white;
  /* ... */
  /* Customize Default Children */
    /* -- component does not have any children components -- */
}
.Logo {
 /*... and use 'var(--Logo_width)' and 'var(Logo_color)' ... */
}
.Logo__image {
 /*... and use 'var(--Logo_width)' and 'var(--Logo_color)' ... */
}
/*... and use 'var(--Logo_width)' and 'var(--Logo_color)' ... */

Navigation Component

:root {
  --Navigation_padding: 5px;
  --Navigation_borderActive: red;
  /* ... */
  /* Customize Default Children */
    /* -- component does not have any children components -- */
}
.Navigation {
 /*... and use 'var(--Navigation_padding)' and 'var(--Navigation_borderActive)' ... */
}
.Logo__image {
 /*... and use 'var(--Navigation_padding)' and 'var(--Navigation_borderActive)' ... */
}
/*... and use 'var(--Navigation_padding)' and 'var(--Navigation_borderActive)' ... */

Menu Component

@import  'logo'; /* default logo component */
@import 'navigation'; /* default navigation component */

:root {
  --Menu_backgroundColor: blue;
  --Menu_fontSize: 15px;
  /* Customize Default Children */
  --Logo_width: 100px;
  --Logo_color: purple;
  --Navigation_padding: 2px;
  --Navigation_borderActive: yellow;
}

.Menu {
  background-color: var(--Menu_backgroundColor)
  font-size: var(--Menu_fontSize)
  /* ... */
}
.Menu__title {
  /* ... */
}

USAGE (of Menu Component)

@import `menu`;

root: {
  /* THEME */
  --ThemeColor1: red;
  --ThemeColor2: yellow;
  --ThemeFocusColor: green;
  --ThemeSize: 20px;
  --ThemeSpacing: 3px;
  /* CUSTOMIZE STYLING */
  --Menu_backgroundColor: var(--ThemeColor1);
  --Menu_fontSize: 15px;
  --Logo_width: var(--ThemeSpacing);
  --Logo_color: var(--ThemeColor2);
  --Navigation_padding: var(--ThemeSpacing);
  --Navigation_borderActive: var(--ThemeFocusColor);
}
@geelen

This comment has been minimized.

Member

geelen commented Aug 15, 2015

I think passing variables down to sub components isn't the right approach. Going back to the original example, I think you're missing one level of separation between parent and child here. Let me start from a "rule" and work backwards:

Each component should style itself

So, Button or Dropdown in your example are separate components and should have their own styles. So no <Button className={styles.Button}>, just <Button>. So { this.props.children } is no issue, since everything styles itself.

Now, of course, you might have several variants of a component. So <Button variant='inline'> or <Button variant='primary'> or whatever. The component still styles itself, you just ask for a particular version.

But I think what you're getting at is something around layout of multiple components. That some parent container will have to say "All the things below me have X pixels padding" or something. That's where I think you're missing a level. Instead of:

return <div className={styles.parent}>
  <Button />
  <DropDown />
</div>

I think you want something like this:

return <div className={styles.parent}>
  <div className={styles.child}>
    <Button />
  </div>
  <div className={styles.child}>
    <DropDown />
  </div>
</div>

Adding the child layer lets you make styling decisions across a full set of children without confusing the separation between the actual components themselves.

@serapath

This comment has been minimized.

serapath commented Aug 16, 2015

What i'd like to do is nest the Button component class in the ButtonGroup so that its styling changes. At the moment
(cited)

@joeybaker mentions:

Some alternative approaches:

  1. Have buttons take a prop that allows their styling to match what a button group would do. This might come close to eliminating the need for a ButtonGroup component.
  2. Have your button group component set its own classnames on its button children. This is kinda "magical" and breaks encapsulation, but might come the closest to what you want.
  3. Create a whole new ButtonGroup component that doesn't use the Button component at all.

(1.) feels to me the closest to having some kind of variables or let's say parameters that an be passed to children

(2.) might also go in that direction...

BUT: Passing parameters (where a child component can have sensible defaults) makes the styling changes to be applied very controllable, because (1.) and (2.) might need knowledge about the styling details of the child to make it behave in the right way, while parameters can be taken care of by the child, to avoid the need for a component user to know implementation details of the child


@oliverbenns See above (BUT: ...). I totally agree with:

Not knowing the children

  1. Children could be a button, but it could also be a dropdown etc ( styles.dropdown , styles.button )
  2. They're not really 'states' of these components, as it's particular to the parent. E.g. adjusting some layout particular that only happens in this parent component.

It might be a good thing to let the child control what kind of styling changes a parent is allowed to do, so the child can specify sensible defaults and make sure a user does not need to know implementation details in order to "adjust" the styling. To me, this sounds like passing styling arguments to a child to adjust the child's layout and appearance in a controlled way.


@ojame, You mention:

<Parent>
   <Button className={styles.buttonWithContextOfParent} />
  <Dropdown className={styles.dropdownWithContextOfParent} />
</Parent>

If it's because you're trying to apply classNames to whatever children are supplied, under all circumstances and with total authority (leaving all of the concern to the parent, which, is probably a bad idea)

To me that feels like static typing would be necessary so that a component could specify a type or some kind of interface in order to enable a parent to know what kind of classNames can be supplied - because given the children have some kind of type, the parent knows what is sensible to apply.

But instead:
I imagine, that when I create a component and pass it some child components, I check their documentation in order to know what kind of styling arguments (e.g. a special font or a certain theme color) I can pass in and then I will just do that. The child would be written in a way, so that it set's sensible defaults and uses the supplied arguments in a controlled way.


@geelen Why do you think:

I think passing variables down to sub components isn't the right approach.

You say:

Each component should style itself

I would also like to see that and passing in arguments puts the child in control, because it can set sensible defaults and otherwise use the supplied arguments in a controlled way and/or decide to not allow access to certain styling properties at all. I imagine it similar to the way a function definition exposes a certain contract (parameters) to a user/caller, where the user can pass in arguments

<Button variant='inline'> or <Button variant='primary'> could already be interpreted as some kind of arguments supplied to the children, but the variety is finite, because a parent can only choose from the options a child offers, which means the author of a child component needs to anticipate all possible usage scenarios.

For example, what about: <Button themeColor='#339933' themeFont='Futura'>
Ideally, the Button would apply certain properties to the Futura font and adapt all the places in which it uses color to match or anticipate the given themeColor.

Regarding your example to create a container for each child, i do something similar in my example:

<!-- Menu.html -->
<div class="Menu">
  <div class="Menu_title"></div>
  <div class="Menu__logo----Logo"></div>
  <div class="Menu__navigation---Navigation"></div>
</div>

The above is my Menu Component, which in JSX would probably be written like:

return <div className={styles.Menu}>
  <div className="{styles.Menu_title}">{this.props.title}</div>
  <div className={styles.Logo}> <Logo /> </div>
  <div className={styles.Navigation}> <Navigation data="{this.props.navigationData}" /> </div>
</div>
@nkbt

This comment has been minimized.

nkbt commented Aug 16, 2015

For example, what about: <Button themeColor='#339933' themeFont='Futura'>

I'll just leave it here https://github.com/petehunt/jsxstyle

@serapath

This comment has been minimized.

serapath commented Aug 17, 2015

+1 great read :-)

I like his example make-style-reuse-easy
Later he goes on to say what-about-re-theming-off-the-shelf-components

you should make the customizable characteristcs part of the component's public API. This may mean passing component classes into reusable components via props. If this starts to get cumbersome, guess what, this is just dependency injection, which is a very well-understood problem and there's lots of tools to help.

In the past, I often ended up maintaining some kind of state that i pass to components,
but next to it i often also had a ui-state which took care of whats currently selected or active or disabled and so on...

In general, it would be nice to have more flexibility than just some kind styling state machine, where I have to anticipate every possible styling state upfront by defining more variant classes

  • To me, any kind of CSS is similar to default values for function parameters in javascript in case no arguments are passed to a function call
  • Later during runtime, arguments might get updated using data binding or similar mechanisms.

The same I imagine for CSS, just having some default values and during runtime, styling can update.

Example:

@nkbt

This comment has been minimized.

nkbt commented Aug 17, 2015

Highly recommend to also watch this talk:
Michael Chan - Inline Styles: themes, media queries, contexts, & when it's best to use CSS
https://www.youtube.com/watch?v=ERB1TJBn32c

Michael talks about separating ui state styles from ui styles. We use the same concept in our projects for a while and it works really well.

@ojame

This comment has been minimized.

ojame commented Aug 18, 2015

I spoke about this a little bit here: JedWatson/react-date-select#14

@serapath

This comment has been minimized.

serapath commented Aug 18, 2015

At Minute 14:49 he gives a code snippet example:

return ( /* ... */
  <li 'todo-list__item'
    style={Object.assign({},
      item.complete && completeItemStyles, // <= styles the component with the "sensible default styles"
      item.complete && this.props.completedItemStyle // <= overwrites with "passed in" style arguments
    )}>
  /* ... */
///////////////////////////////////////// so later you use it like ...
<TodoList
  items={this.state.items}
  completedItemStyles={{ fontStyle: 'italic' }} />

which is what I tried to say before. It's passing in custom styles via arguments to the "component function", which in the syntax above is written in html instead of javascript.
In Javascript it would probably look something like

function TodoList(items, completedItemStyles) {
  return // dom or virtual dom
}
TodoList(this.state.items, { fontStyle: 'italic' })

He summarizes it in the presentation with:

What we did?

  • State is fully owned by our component
  • Stateful classes removed from DOM and CSS
  • CSS reduced to "appearance"-only
  • Styles can be overriden at the call site
  • Better specs

I think the bold bullet points again try to make that more clear.

@serapath

This comment has been minimized.

serapath commented Aug 18, 2015

@ojame
I checked the example in your link

<button
   onClick={this.props.onSelect}
   className="DateSelectFooter__button DateSelectFooter__button--primary"
>
   Confirm
</button>

Here a classNames are passed in. My thought about that is, that depending on the styling attached to that class

  1. it might break or not break the other styling defaults on the button component if you have all the freedom to redefine the attached styling.
  2. or you have to select from the different styling variants (e.g. --primary) that are defined by the component

I imagine, there are certain style attributes in any given component that could be easily and safely changed, like for example a color or a border-radius, but there are other properties that could break the component by overwriting something related to the layout.

So if you mix arbitrary components, the one thing that knows what kind of styling you can safely replace and whats not meant to be replaced is the component itself, thus the component should specify certain styling attributes that it allows to be replaced.

Imagine

.MyComponent {
  color: #f00
  border-color: #f00
}

Does that mean border-color and color should always have the same color or can they be different?
Maybe in some cases they should always be the same, because the author of the component has the intention to have it like that in order to keep some style consistency - in other components maybe it should be possible.

So there is a problem in the following code:

let classNames = {
   footerButton: 'DateSelectFooter__button',
   footerButtonPrimary: 'DateSelectFooter__button--primary',
}

var DateSelect = React.createClass({
   componentWillReceiveProps(props) {
      classNames = Object.assign({}, classNames, props.classNames);
   }

   render() {
      return (
          <DateSelectDialog classNames={classNames} />
      )
   }
});

because it requires knowledge about the above mentioned constraints. Maybe you want to switch the button color, but instead of telling the component to change one property that the author declared to be customizable, like the color, you now have to overwrite all internal classes of that component that make use of color to get what you need. So you need to dig into the component and figure out what to overwrite and where... right?

@ojame

This comment has been minimized.

ojame commented Aug 18, 2015

We compose:

node_modules/component/style.css

.DateSelectFooter__button {
  background-color: transparent;
  background-image: none;
  border: none;
  border-radius: 0;
  color: #666666;
  float: left;
  font-size: inherit;
  outline: none;
  padding: 8px;
  text-align: center;
  width: 50%;
}

app/partials/component.css

.footerButton {
  composes: DateSelectFooter__button from "node_modules/component/style.css";
  background-color: red;
}

app/partials/component.js

import styles from './component.css';

<DateSelect
   classNames={
      footerButton: styles.footerButton,
   }
/>

It's not up to the author of the component to hand-hold every implementation under the sun (ie, ensure, no matter what the user styles, that it always "works"). If there are some absolutely required styles, maybe inline them on the component so it makes it harder for an implementation to override them. That's besides the point though - the above example shows you how to override one property while retaining the others.

@serapath

This comment has been minimized.

serapath commented Aug 19, 2015

/* ... */
   componentWillReceiveProps(props) {
      classNames = Object.assign({}, classNames, props.classNames)
/* ... */
      return (
          <DateSelectDialog classNames={classNames} />
/* ... */

So the classNames contain the default styling and the Object.assign adds/overwrites styling based on props.classNames. How do I know what I want to overwrite?

You write

.DateSelectFooter__button {
/* ... */
}

and

.footerButton {
  composes: DateSelectFooter__button from "node_modules/component/style.css";
  background-color: red;
}

I guess the goal or need is to sometimes theme and sometimes re-style a component from scratch.
I imagine, I need to know the implementation details of the DateSelectFooter component to know where exactly to "pass in" or "overwrite" background-color, so that in complex components I really manage to adapt all the "backgroundish" parts to red.

Rather than passing in classNames, I'd like to see passing in properties that gets put into all kinds of appropriate positions.

WHY?
Because in the video from @chantastic he mentions you should re-use, but not re-purpose components. So maybe a "best-practice" would be, to open basically most or all of the css properties through parameterization and allow to pass in "style attribute" arguments which will be used in one or many positions.
If I'm using dozens or hundreds of components to make up my app and they are well maintained and optimized components, I have a lot less work If I can just pass in arguments and get otherwise sensible defaults without having to think about the styling internals of the many visual components i use.

If I really need to do something that wasn't intended, maybe I would just fork a visual component that comes close, modify it and use that one instead, right? Because after all it should be fairly easy to do that...

@serapath

This comment has been minimized.

serapath commented Aug 19, 2015

What about:

:root {
  --defaultColor: 'red';
  --defaultRadius: '5px';
}
/* components/submit-button.css */
.normal { /* all styles for Normal,
  might use var(--defaultColor) and/or var(--defaultRadius)
*/ }
.disabled { /* all styles for Disabled,
  might use var(--defaultColor) and/or var(--defaultRadius)
*/ }
.error { /* all styles for Error,
  might use var(--defaultColor) and/or var(--defaultRadius)
*/ }
.inProgress { /* all styles for In Progress,
  might use var(--defaultColor) and/or var(--defaultRadius)
*/

and

/* components/submit-button.js */
import styles from './submit-button.css';
// styles would be a function object with classname properties, e.g. styles.normal
console.log(styles) // serves as documentation to know which attributes you can customize
/* =>
function customize (args) { // a generated closure
  var color = args.color || defaultColor // e.g. 'red'
  var radius = args.radius || defaultRadius // e.g. '5px'
  // ... and more ...
  updateStyling(color, radius/*, ... */)
}
*/
var theme = { color: '#3c6', radius: '0px' }
styles(theme)
buttonElem.outerHTML = `<button class=${styles.normal}>Submit</button>`

In a Component, each rendering could change the color or radius dynamically.

@webOS101

This comment has been minimized.

webOS101 commented May 27, 2016

Has anyone developed a good solution for this problem? It seems people talked all around the problem but never really addressed the original concern.

@TrySound TrySound closed this May 31, 2017

@todesstoss

This comment has been minimized.

todesstoss commented Jan 3, 2018

Not sure about good, but here is my solution to this issue:
write styles in your component which is rendering unknown children, but apply 'em in your container component, just importing styles as regular:

your container:

import Nav from '../../components/Nav';
import NavStyles from '../../components/Nav/Nav.scss';
~
<Nav>
<div>whatever</div>
// here we apply class
<ReservationButton className={NavStyles.NavButton}>
</ReservationButton>
</Nav>

your component css:

.NavButton {
  // some special styles which are related to any button only inside my nav components
  position: absolute;
}

and finally your child component:

function ReservationButton({ children, className }) {
  const buttonClassName = `${className} ${styles.ReservationButton }`;

  return (
    <button className={buttonClassName}>{children}</button>
  );
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment