Skip to content
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

Add storybook css interface (4/7) #164

Merged
merged 2 commits into from
Jul 3, 2018

Conversation

philipfweiss
Copy link
Contributor

@philipfweiss philipfweiss commented Jun 29, 2018

Adds storybook:css command to package JSON which instantiates the storybook with react-with-styles-css-interface.

Builds off of
#162
#163

@philipfweiss philipfweiss changed the title Add storybook css interface Add storybook css interface (5/7) Jun 29, 2018
@philipfweiss philipfweiss changed the title Add storybook css interface (5/7) Add storybook css interface (4/7) Jun 29, 2018
@philipfweiss philipfweiss changed the base branch from master to pw--clean-up-rheostat-CSS-theming June 29, 2018 23:28
@@ -770,7 +765,6 @@ class Rheostat extends React.Component {
ref={this.setHandleContainerNode}
{...css(
styles.handleContainer,
handleContainerStyle,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that this didn't actually do anything, and was just being overridden. Since it is not overridden with the CSS interface, and was causing graphical issues, I removed it.

css/styles.css Outdated
@@ -0,0 +1,113 @@
.DefaultProgressBar__vertical {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this file be gitignored (altho not npm-ignored), and generated as part of build output?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm what is the rationale for not putting it on Github? On one hand it's a generated file, but on the other it might be useful for someone skimming the repository online or right after cloning.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally those aren't supported use cases; you can use http://unpkg.com/react-dates/ if you want to browse generated output.

css/styles.css Outdated
width: 15px;
top: 0px;
height: 100%
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all files should have trailing newlines

package.json Outdated
@@ -14,6 +14,7 @@
"prepublish": "npm run build",
"lint": "eslint --ext .js,.jsx src test stories",
"storybook": "start-storybook -p 9001",
"storybook:css": "npm run build:css && start-storybook -p 6006 -c .storybook-css",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"prestorybook:css": "npm run build:css"?

registerCSSInterfaceWithDefaultTheme();

function loadStories() {
require('../stories/ExampleSlider.jsx');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should omit the extension

@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch from e63b09e to de87561 Compare July 1, 2018 21:13
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch 2 times, most recently from b648666 to f1ddaa2 Compare July 1, 2018 21:29
@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch from de87561 to daedb98 Compare July 1, 2018 21:33
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch 2 times, most recently from 2d0f1e8 to 3a0e8ce Compare July 1, 2018 21:41
@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch 2 times, most recently from cf46dd1 to 8cd1c2f Compare July 1, 2018 21:58
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch 3 times, most recently from 1cbcb92 to 532c167 Compare July 1, 2018 22:03
@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch from 8cd1c2f to cdf7134 Compare July 1, 2018 22:05
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch from 532c167 to 3121296 Compare July 1, 2018 22:06
@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch 2 times, most recently from 4cfda86 to e70531a Compare July 2, 2018 22:22
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch from 3121296 to 64e6b9c Compare July 2, 2018 22:35
@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch 3 times, most recently from 926dc37 to d39970f Compare July 2, 2018 22:59
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch from 64e6b9c to 1a0d6c4 Compare July 2, 2018 23:01
Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of comments first

.gitignore Outdated
@@ -7,6 +7,7 @@ coverage
build
css/rheostat.css
.nyc_output
css/styles.css
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't we change this naming?

package.json Outdated
@@ -71,6 +73,7 @@
},
"dependencies": {
"airbnb-prop-types": "^2.10.0",
"clean-css": "^4.1.11",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, not a dep, let's remove this package from here, it's already in dev deps

@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch from 1a0d6c4 to eedb9a2 Compare July 2, 2018 23:42
Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@philipfweiss philipfweiss force-pushed the pw--clean-up-rheostat-CSS-theming branch from d39970f to 8eaa55e Compare July 3, 2018 00:06
@philipfweiss philipfweiss force-pushed the pw--add-storybook-css-interface branch from eedb9a2 to f5bd378 Compare July 3, 2018 00:09
@philipfweiss philipfweiss changed the base branch from pw--clean-up-rheostat-CSS-theming to master July 3, 2018 01:26
@philipfweiss philipfweiss merged commit 391aadf into master Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants