CSS in JS Transition: First Step by tajo · Pull Request #77 · cloudflare/cf-ui

tajo · 2017-02-23T01:55:57Z

What's new

update (pretty much rewrote) cf-component-button
add a new component cf-style-container, it is a wrapper for some fela APIs so we can keep the rest of components fela free
add a new build process for the style guide
- it lives in the /styleguide dir
- it's a full-fledged app using webpack2
- it directly imports components from /packages/cf-xxx-xxx/src/xxx.js
- you can do local deving with hot module replacement via:
```
npm start
open http://localhost:8000
```
- you can get static version (that will be published) via:
```
npm run examples:new
open build/index.html
```
- it's using a webpack loader to create and highlight code examples
- it's using webpack aliases so we don't have to use relative paths in the code examples

What's broken

We need to approach tests differently since classNames are dead (auto-generated). I want to setup Jest and snapshot testing. Right now, I commented out the test for cf-component-button.
We have to ignore new components in old gulp tasks since it wouldn't work (missing styles and broken paths since it's using browserify). So you can still build old style guide but without transitioned components.

What's next

This PR is pretty rough but cf-component-button won't change much. I will have to do some updates so it supports our internal styles.
The new style guide is a POC right now:
- First, I want to match the look and functionality of the current one. I'll have to create another webpack loader to extract propTypes. (we are moving gulp code into webpack loaders, how cool is that?)
- I will create a CLI tool that will autogenerate the whole /styleguide folder. So we can easily use it for private cf-ui as well. The same approach as with doca.
- I might add some new features for designers (think of live editing of themes).
I'll create @cloudflare/cf-component-button

…c styleguide based on webpack2

…uide for now)

tajo · 2017-02-23T02:08:12Z

Please ignore linting. I'll switch cf-ui to prettier soon, so I didn't care.

hturan · 2017-02-23T11:21:25Z

This is ace, thanks for putting it up for review! I haven't had a chance to go over the fela re-write yet, but have some notes concerning the styleguide.

It's worth noting that there's currently an internal project to create a styleguide, so I'd hold off creating anything too revolutionary for now. I'm all for refactoring into a styleguide app (in fact, that's exactly what I did when I created my dynamic styleguide proof-of-concept; making a webpack app in /styleguide directory), but maybe just duplicate the functionality and design we have now (but using webpack instead of gulp) 👍

Also FWIW, as part of the dynamic styleguide I made (the one that gave you editable PropTypes via HTML inputs) I created a PropTypes webpack loader that used react-docgen to process the raw component source. It was pretty hacky and failed on a few components due to weird dependencies, but I'm happy to send you the source if you're interested. For now, it might be worth just taking the source parsing logic we have in the gulpfile and moving it into a loader. Let me know if you need a hand with this!

wyuenho

This is awesome. I see some short hands used but I guess they are allowed since it looks like fela just uses the DOM's style properties. It's been a while since I've used them, it's a bit surprise to rediscover that some CSS properties don't support shorthands, but some do. Is this still a concern?

Just a few question, since we've moved all docs to go under /docs so Github Pages can serve them, this styleguide is intended to replace that at some point right? Also, now that webpack is in place, this means we can conceivably remove a hugh chunk of code from the gulp file that has to do with the examples, and pave the way to use webpack to compile all the components in the future right? Can you write down your plans on the Webpack ticket?

wyuenho · 2017-02-23T14:16:19Z

package.json

  },
  "devDependencies": {
    "autoprefixer": "^6.3.6",
+    "babel-cli": "^6.23.0",


Why do you need this?

Good catch. I'll remove it.

jwineman · 2017-02-23T17:03:41Z

packages/cf-component-button/test/Button.js_

@@ -0,0 +1,46 @@
+const React = require('react');


Why the trailing underscore on packages/cf-component-button/test/Button.js_?

outdated nvm

tajo · 2017-02-23T19:05:05Z

@hturan It seems we are working on the same thing. Can you send me a link? To make it dynamic (with HMR) it needs to be a part of cf-ui repo. Only this way, you can have the best dev experience (no rebuilding, no refreshing). I think we should merge everything into the styleguide app. I'll bring it up to the current functionality. We can start adding new features later (live editing through HTML inputs would be really cool).

For now, it might be worth just taking the source parsing logic we have in the gulpfile and moving it into a loader.

That's exactly my plan.

@wyuenho You can use shorthand properties or longhand properties, but you can't combine them in the same rule. I think we should mostly avoid shorthand properties (especially for common properties as margin, border or padding). It adds some extra code but it will cut the size of CSS output in a long term since longhand properties are more unique and can't benefit from Atomic design that much. Also, it makes customization easier (if I used shorthand margin for the button component, its rule function would be a mess). I don't have a strong opinion on this yet. Let's see what our experience will be but not using shorthands definitely will not hurt.

Ad webpack. Yes! Once we transition all components, we can remove gulp completely. Examples are handled by /styleguide (notice that big chunks of gulpfile code are replaced by webpack loaders) and build can be done with a npm script/webpack too.

wyuenho · 2017-02-23T22:29:12Z

packages/cf-style-container/src/index.js

+	createComponent as createFelaComponent
+} from 'react-fela';
+
+export const createComponent = (rule, type = 'div', passThroughProps = []) =>


let's call this node or element, type is overused

That's what fela is using now so I want to match their API/terminology if possible. Node / Element has meaning in DOM terminology. This actually accepts React components as well.

tajo added 5 commits February 22, 2017 17:32

rewrite of cf-component-button, add cf-style-container, add some basi…

146b4c9

…c styleguide based on webpack2

add example code for cf-component-button

1318f9d

add static build for styleguide

e4bea19

fix eslintrc (we will soon switch to prettier so I don't lint /styleg…

54fc3ed

…uide for now)

exclude transitioned components from gulp tasks building examples

78f5661

tajo requested review from hturan, jculvey, jwineman, koddsson, marksteyn, nkcmr, toekneestuck and wyuenho February 23, 2017 01:55

enable uglify for prod build

9434297

tajo requested a review from terinjokes February 23, 2017 02:08

wyuenho reviewed Feb 23, 2017

View reviewed changes

jwineman reviewed Feb 23, 2017

View reviewed changes

jwineman approved these changes Feb 23, 2017

View reviewed changes

remove babel-cli

f8ce54d

wyuenho reviewed Feb 23, 2017

View reviewed changes

tajo merged commit 1a15512 into cloudflare:master Feb 24, 2017

hturan mentioned this pull request Mar 1, 2017

Missing react-fela in cf-component-button 4.0.0 #82

Closed

jwineman mentioned this pull request Mar 16, 2017

Fela Migration #100

Closed

33 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSS in JS Transition: First Step#77

CSS in JS Transition: First Step#77
tajo merged 7 commits intocloudflare:masterfrom
tajo:fela

tajo commented Feb 23, 2017

Uh oh!

tajo commented Feb 23, 2017

Uh oh!

hturan commented Feb 23, 2017 •

edited

Loading

Uh oh!

wyuenho left a comment •

edited

Loading

Uh oh!

wyuenho Feb 23, 2017

Uh oh!

tajo Feb 23, 2017

Uh oh!

tajo Feb 23, 2017

Uh oh!

jwineman Feb 23, 2017

Uh oh!

jwineman Feb 23, 2017

Uh oh!

tajo commented Feb 23, 2017

Uh oh!

wyuenho Feb 23, 2017

Uh oh!

tajo Feb 23, 2017 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tajo commented Feb 23, 2017

What's new

What's broken

What's next

Uh oh!

tajo commented Feb 23, 2017

Uh oh!

hturan commented Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wyuenho left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wyuenho Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

tajo Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

tajo Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

jwineman Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

jwineman Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

tajo commented Feb 23, 2017

Uh oh!

wyuenho Feb 23, 2017

Choose a reason for hiding this comment

Uh oh!

tajo Feb 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hturan commented Feb 23, 2017 •

edited

Loading

wyuenho left a comment •

edited

Loading

tajo Feb 23, 2017 •

edited

Loading