RFC Drop scss in favor of styled-components

[mcnuttandrew]

As anyone familiar with this github repo is aware, one of the most frequent bugs folks encounter is not importing the styles. As anyone familiar with developing in this project is aware, a frequent annoyance is rebuilding node-sass all of the time.

I propose that we address these problems by rewriting the style sheet as a collection of minimal styled components. There is a modest amount of cruft in the the style sheets which could easily be taken out in this process. I like the library called styled-components, but i'd be open to other solutions as well.

Interested to hear comments!

[lzhuor]

+1 👍

To migrate to styled-components, 5 wrappers are needed:

• [ ] Plot (plot.scss)

• [x] Legend (legend.scss) : I made a POC on Legend components - Pull Request #825

• [ ] Radial Chart (radial-chart.scss)

• [ ] Tree Map (treemap.scss)

• [ ] Examples Page (examples.scss) ? TBD

[mcnuttandrew]

I'm not sure the examples page needs to be moved to styled-components. Like it is lowest priority as it is not externally consumed.

[lzhuor]

Hi @mcnuttandrew , I agree that Examples Page is not externally consumed and can be put as the lowest priority. Re-arranged its order.

The reason why I put it on the TODO list is that examples.scss consumes a few variables imported from main.scss.

For example:

&.left-bottomEdge {
        border-left: 2px solid $hint-color;
        left: -1px;
        height: $margin-bottom;
        width: 0;
        top: -$margin-bottom;
      }

$margin-bottom and $hint-color are used directly as an imported var. If we would like to remove main.scss together with plot.scss etc., we probably have to migrate examples.scss or we can dump all SCSS variables into examples.scss to resolve the issue.

[jckr]

Hey Andrew,

IMO, css-in-js solutions like styled-components and stylesheets based solutions like scss are not exclusive.

In the context of react-vis, which can potentially create thousands of DOM elements, css is likely to be more performant than inline styles. Another thing in favor of a css solution is that it's always possible to override css with css, and often with injected styles if the right properties are exposed. Conversely, it's not always possible to override inline styles with css and/or with injected styles. Finally, a great side effect of using css is that DOM elements get classes which are helpful for testing. Now a major issue for a css-based solution is that css/scss needs to be imported, which is often, at least, impractical. It can also be argued that inline styles are more consistent with the spirit of React.

With that in mind, a good solution:

• would let users style their charts (duh),
• wouldn't require people to import CSS/SCSS, but could let them use it;
• wouldn't require people to use styled-components (or any specific css-in-js library), but could let them use it;
• would allow fine, granular control of the look and feel of charts and their elements,
• wouldn't require too much overhead.

Though we could go even further and let people completely reskin their charts with a consistent theme without having to manually override dozens of individual rules.

With that in mind, the direction I was considering is:

XYPlot can take a theme prop which is a json object where properties are either css rules or correspond to the class of an XYPlot element. ie:

{ 
  fontFamily: "serif",
  lineSeries: {
    stroke: "black"
  }
}

XYPlot would then pass this theme prop to its children. The children will then take this theme into account when styling (ie explicit style prop > other props > theme prop > css rules).

Further, react-vis will propose:

• several ready-to-use themes complete with colors, spacing/sizing, etc. themes can also govern margins.
• a StyledXYPlot which will have a theme equivalent to the CSS style sheet. ie it would look identical to XYPlot with imported CSS, only it wouldn't need to import the stylesheet.

[lzhuor]

Hi @jckr and @mcnuttandrew ,

How about importing SCSS into the components directly with webpack scss loader?

Example:

ContinuousColorLegend component:

import '../styles/legend.scss';

It resolves the problem where user forgets to import scss but doesn't necessarily require styled-components.

[jckr]

This already works but on webpack. We can't force anyone who wants to use react-vis to use webpack (i mean we could, but I don't think we should)

[mcnuttandrew]

Many thoughts!

• Switching to something like styled-component is less severe than you think! I think the way that styled-components works is by injecting adhoc styles into css and then injecting selecters into the jsx to match those adhoc styles. That way things are still performant without using inline styles. (To be clear: I am not arguing for inline styles. I think we already perhaps a few too many inline styles! )

• In my experience styled-components components play nice with other styling systems and it is not incredibly difficult to over-ride the styles, again because those styles are inserted via classes. I think supporting node-scss and is really painful and adds lots of cruft to the library. I think styled-components would stream line the adoption process.

• I think your idea about offering a pre-styled XYPlot is interesting, but I think it would be better to have a stronger default that users can opt out of easily (ie dis-engage some of the styles). I think having themes will end up being a one (or several) size fits none result.

• An additional benefit of the proposed stylification process is that we can rewrite the style sheet to be more minimal and less opinionated, which will still allow easy user over-rides.

[jckr]

we have strong proponents of styled-components at Uber. kepler.gl for instance uses them extensively.

an additional aspect of the future system which isn't too expensive to consider is component injection. We could have something like <XAxis components={{AxisTitle: MyAxisTitle}} /> where MyAxisTitle replaces AxisTitle for all intents and purposes. MyAxisTitle could be a completely new component, or just be a styled version of AxisTitle.

[mcnuttandrew]

Should this process be undertaken it would be a good moment to develop a snapshot testing. Having seen the wonders of puppeteer, I can testify that it's kinda easy to do snap shot testing now, and we should be doing it. So if were were to execute a css-in-js strategy a plan of attack would be like:

1. Write tests to verify the look and feel of all components (single snapshot per showcase item would be fine)
2. Transition scss styles to styled components
3. Release and bug catch

[wangzuo]

I have created react-vis-styles to help me avoid bundling css.