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

feat: add Storybook #66

Merged
merged 1 commit into from Oct 1, 2019
Merged

Conversation

riccardo-forina
Copy link
Collaborator

@riccardo-forina riccardo-forina commented Sep 23, 2019

Screen Shot 2019-09-24 at 6 05 06 PM

Copy link
Collaborator

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

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

Very useful for a lot of projects, thank you!!

It looks good, I left a few questions. Other than those the only things I wanted to ask about is adding storybook-static to the gitignore and if you know what gives on either of these warnings or error?

Screen Shot 2019-09-24 at 5 31 02 PM

Screen Shot 2019-09-24 at 6 04 48 PM

I get the error when resizing the browser window down to a small viewport. It looks like those breakpoints might be customizable, is there a way to align the storybook breakpoints with those defined in patternfly?

Awesome work!

package.json Outdated
"clean": "rimraf dist"
"clean": "rimraf dist",
"storybook": "start-storybook -p 6006",
"build-storybook": "build-storybook"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we change it to build:storybook, and maybe update the existing build script to be build:app, that way they all follow a same pattern?

package.json Outdated Show resolved Hide resolved
.storybook/webpack.config.js Show resolved Hide resolved
package.json Show resolved Hide resolved
@riccardo-forina
Copy link
Collaborator Author

Thanks for the feedback! The warning bit was tricky, Storybook's own Webpack's config was providing some default CSS loader that wasn't compatible with the one needed for PatternFly.

@seanforyou23 seanforyou23 added the enhancement New feature or request label Oct 1, 2019
Copy link
Collaborator

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

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

Nice work @riccardo-forina - looks great 💙 I checked and that error about active prop is a known issue - storybookjs/storybook#5910 - which has been reopened, so hopefully, we'll just inherit the fix at some point once it's posted.

Thanks again, this will surely save people some time on the initial setup 👍

@seanforyou23 seanforyou23 merged commit 68c063a into patternfly:master Oct 1, 2019
bhanvimenghani pushed a commit to bhanvimenghani/patternfly-react-seed that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants