Autodesk / hig

Autodesk's unified design system
https://storybook.weave.autodesk.com
Apache License 2.0
181 stars 114 forks source link

Enable overriding stylesheet for themable components #1210

Open nfiniteset opened 6 years ago

nfiniteset commented 6 years ago

As a developer I can override the style of an individual component instance so I can tweak it as necessary


I think we will likely be asked to support this, and I think it's a reasonable thing for teams to expect. That said I have some concerns about it.

My main concern is that it represents a huge new API surface area. The more surface area we expose, the less flexible/more breaky our library gets.

Shotgun does something like this

function myInputStylesheet(stylesheet, props, themeData) {
  return {
    ...stylesheet, input: {
        ...stylehseet.input, 
        backgroundColor: props.hasFocus ? "blue" : "transparent"
      }
    }
  }
}

function MyInput(props) {
  return <Input stylesheet={myInputStylesheet} {...props} />
}

Dev notes

For reference, A subcomponent of Flyout supports this approach and Tooltip uses it.

nfiniteset commented 6 years ago
nfiniteset commented 5 years ago

Four approaches to one-off component styles

Override styles via stylesheet function prop (used internally today)

Pass a function to the stylehseet prop. It receives the output of the internal stylesheet function, props, and theme data.

Pros

Cons

Example

function myButtonStylesheet(stylesheet, props, themeData) {
  return {
    ...stylesheet, button: {
        ...stylehseet.button, 
        margin: "20px"
      }
    }
  }
}

function MySpecialButton(props) {
  return <button stylesheet={myButtonStylesheet} {...props} />
}

Enable passing arbitrary className

Set your own classname via prop and write CSS to override styles.

Pros

Cons

Example

<Button className="my-special-button" />
.my-special-button {
  margin: 20px;
}

Style against stable classNames

All components could come with stable classnames meant to be used for styling, unit tests, or other selection needs. The library would apply no styling to these classNames.

Pros

Cons

Example

.my-special-button > .hig-button {
  margin: 20px;
}
<div className="my-special-button">
  <Button title="Send" />
</div>

Pass style prop (generally supported today)

Just pass the override as a style prop.

Pros

Cons

Example

<Button style={{ margin: "20px" }} />
nfiniteset commented 5 years ago

Neo says

I don’t have a chance to use HIG in product yet, but from my perspective & practice, #2 is really useful while building ui layout, as we have so many different products, HIG just can’t cover all the use cases. Without #2 we just can’t use styled-components or emotion to extend the component

Ivan says

As discussed on Friday, I think that a mix of Enable passing arbitrary className and Style against stable classNames is the ideal in my eyes.

Can do basic stuff with a simple class, and more complex stuff if familiar with the internals.

4 makes sense if we’re wrapping all HIG components in our own components. Otherwise, you end up with styles all over your codebase.

nfiniteset commented 5 years ago

Style against stable classNames would still require you to wrap a component in another element in order to select it for a one-off style override.

// Set style on all Buttons
.hig-button {
  margin: 20px;
}

// Set style on one Button
.my-special-button > .hig-button {
  margin: 20px;
}
<div className="my-special-button">
  <Button title="Send" />
</div>
Jovani commented 5 years ago

Aye, but if HIG passed down the classNames prop to the presenter, then we could use a combo of the given className + stable classNames to style.

nfiniteset commented 5 years ago

@Jovani Ah ok, so would you say the stable classnames would be useful mostly to target internal elements?

Maybe a simple but typical use case would look something like this:

Example of markup output by Input component

<div className="hig-input my-special-input">
  <input className="hig-input__input" />
  <div className="hig-input__halo" />
</div>
.my-special-input > .hig-input__input {
  margin: 20px;
}