[Proposal] CSS in JS documentation

[jkup]

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).

Fix #5204

I've been talking with @bbondy and @bsclifton for a while about refactoring the CSS in browser-laptop. I think that a modular, component based approach would suit us well. This PR is a proposal for adding Aphrodite to our codebase and moving in the direction of writing our CSS in JavaScript.

Attached is the dependency and a rough draft with my thoughts around adopting Aphrodite. Some things that would be great to discuss in this PR:

• Are there other approaches people prefer? Radium? PostCSS?, Sticking with SASS/LESS?
• The big win here is lightweight colocation of our CSS and JS. This comes at the cost of some nice to haves like global variables and some nice tools like Stylelint (which only works with PostCSS)
• How should we go about implementing? It makes sense to me to just take an incremental approach. The only downside here is there could be conflicts between the generated styles and the ones inherited from classNames.
• Anything else I'm missing!

Migration

I think the best way to migrate would be one component at a time. Basically since Aphrodite uses className we would just have components the either use CSS classes or Aphrodite classes, never both.

There is a small issue of collisions. Aphrodite generates unique classes so we won't have collisions that way but we still have lots of element based styling (.SettingsItem span {}) so even with removing classNames elements might still be inheriting styles from our .less files.

Shared Styles

I really like the approach @bbondy outlined of making a styles folder with some shared.js files that just export styles. So we could have dialog.js which just exports all shared dialog styles and then require it in each dialog component (overriding specifics if needed).

Sizing units

I think the best approach moving forward would be to use relative sizing units instead of pixels.

Text

I really like the approach that CSS-Tricks takes which is to use px at the document level, em's for all text elements (h2, p, etc), and rem for actual components (sortableTable, navigation, etc). That would look something like this:

html {
  font-size: 14px;
}

.sortableTable {
  /* Components are relative to the document size */
  font-size: 1.2rem;
}

.sortableTable h2 {
  /* Specific text elements, if necessary, are styled relative to their component */
  font-size: 1.4em;
}

Elements

Beyond font sizes, I'm still trying to figure out best approach for width and height of elements. You can use relative sizes like em's but I'm not sure if that's confusing or helpful.

Aphrodite vs. inline style objects

With React, you can simply pass JavaScript objects into the style attribute of any JSX element. There are two big advantages to Aphrodite over this approach.

1. Generated classes allow much easier debugging with devtools over pure inline styles.
2. Aphrodite allows for media queries, pseudo selectors and hover/visited state.

Extra Reading

In case it helps, I wrote a blog post describing how we could share styles and made a demo project in case anyone wants to mess around with Aphrodite!

[bsclifton]

Read through this (paying attention to the examples and considering how we'd fit this into the project). I really like it! Some of the PROs which are huge, IMO:

• Plays along very well with React ( 😄 )
• Styles are encapsulated (and are inheritable) by the components.
  • knowing which file to edit is easy
  • it seems that the library would namespace the objects to eliminate unintended cross-component selector matching (which is great! no unintended changes when updating styles)

I'm curious what the downsides are regarding tooling?

[jkup]

The biggest drawback that I see is that Aphrodite is very lightweight and doesn't do any AST transforms. If we took the AST approach and used something like PostCSS there are all sorts of tools (linting, property order, CSSNext features) etc you can tap into.

[bbondy]

The only downside here is there could be conflicts between the generated styles and the ones inherited from classNames.

Can you check what the likelihood of that is? Maybe the generated ones are so obscure that the likelihood of a conflict would be 0?

[bbondy]

Side comment is there is AST being done and linting, but they are just in JS now, but I agree they aren't as in depth as could be done if it wasn't in JS.

[bbondy]

How about for example our Dialogs.
https://github.com/brave/browser-laptop/blob/master/less/forms.less#L100
https://github.com/brave/browser-laptop/blob/master/less/forms.less#L144
https://github.com/brave/browser-laptop/blob/master/less/forms.less#L194
...

It's a lot of repeated styles and could benefit from shared re-use. I'm thinking in those cases we jus require in a shared common style from app/renderer/styles/*.js?

[bbondy]

Prefer something like this for the example in one line:

const {StyleSheet, css} = require('aphrodite')

[bbondy]

Side note is we could probably hook this easily in the future if KA didn't already do this and inspect the style properties and strings and do certain checks and console.warn for problems.

[bsclifton]

@bbondy that is a great point- we should be able to do testing for important styles 😄

[jkup]

Yeah totally. My only point with "linting" was that Aphrodite doesn't have a robust ecosystem of tools available like PostCSS does.

[bbondy]

Can you make a recommendation on units as well as part of this documentation? Should we avoid px for example on new stuff or is it ok? We seem to have some support issues along the lines of things look tiny, maybe due to dpi? Not sure but just wondering if this type of thing could be handled at the same time as we do a new style system. Now would be a great time to figure that type o thing out and make sure new things use the new system.

[bbondy]

Also I'd love to hear about benefits / contrast to if we just used react inline style={obj} in contrast. I know in general inline styles are bad, but not sure why in react land in contrast to this. I'm sure there are benefits but just would like to read about what they are.

[bsclifton]

Ready for merge after you put your last batch of notes, @jkup 😄