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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storybook: Try to integrate the playground into Storybook #18191

Merged
merged 11 commits into from Nov 12, 2019

Conversation

@gziolo
Copy link
Member

gziolo commented Oct 30, 2019

Description

Part of #17973.

This PR explores how likely it is that the playground could work inside Storybook.

So far, I'm positive, I have found two issues but I guess they are both solvable. I need to play a bit more with the version that is controlled by Parcel as I see that some core blocks explode after they get inserter, e.g. Heading. This is more concerning. - all issues discovered are resolved now and the one about broken blocks isn't related to this PR.

Updated docs:
https://github.com/WordPress/gutenberg/blob/1c440e1805f354ecc5d7d8e2deaf977013473755/docs/contributors/getting-started.md#storybook

How has this been tested?

npm run storybook:dev

npm run storybook:build

Screenshots

Screen Shot 2019-10-30 at 1 37 20 PM

Known issues

All of them are resolved now 馃帀

  • Fonts are missing
  • Playground specific styles are missing because of issues with importing SASS files
  • Many blocks explode just after they get inserted, it needs to be further investigated see #18191 (comment), this isn't related to Storybook at all
  • It doesn't integrate with Netlify because of the changed output directory
  • parcel needs to be removed as it becomes obsolete with all these efforts

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
@gziolo gziolo self-assigned this Oct 30, 2019
@gziolo gziolo force-pushed the udpdate/storybook-playground branch from 2650d14 to 426dfd1 Oct 30, 2019
.storybook/playground/index.js Outdated Show resolved Hide resolved
.storybook/playground/index.js Outdated Show resolved Hide resolved
@ItsJonQ

This comment has been minimized.

Copy link
Contributor

ItsJonQ commented Oct 30, 2019

@gziolo Thank you for getting this started 馃槏 .
I can give the CSS and font stuff a try if that's okay with you

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 30, 2019

@gziolo Thank you for getting this started 馃槏 .
I can give the CSS and font stuff a try if that's okay with you

Feel free to push as many commits as necessary to make it shine :)

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 30, 2019

By the way, I have just confirmed that exploding blocks aren't specific to Storybook integration:

Screen Shot 2019-10-30 at 18 23 04

Screen Shot 2019-10-30 at 18 24 29

@ItsJonQ

This comment has been minimized.

Copy link
Contributor

ItsJonQ commented Oct 30, 2019

@gziolo It works! 馃槏

Screen Shot 2019-10-30 at 1 37 20 PM

@gziolo gziolo force-pushed the udpdate/storybook-playground branch from ec71b63 to 1c440e1 Oct 31, 2019
@gziolo gziolo marked this pull request as ready for review Oct 31, 2019
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 31, 2019

This should be ready for review.

I updated docs in 1c440e1 to replace occurrences of the playground with Storybook. See:

https://github.com/WordPress/gutenberg/blob/1c440e1805f354ecc5d7d8e2deaf977013473755/docs/contributors/getting-started.md#storybook

I also decided to keep the old output folder and npm script aliases for the playground as it integrates with Netlify. @epiqueras, what is necessary to adjust it on Netlify side? We can do it at the very end or in a follow-up PR. Whatever you prefer.

.storybook/webpack.config.js Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@gziolo gziolo force-pushed the udpdate/storybook-playground branch from 8876623 to 7fae32c Oct 31, 2019
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 31, 2019

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 31, 2019

The benefit of using Storybook, you get so many stuff for free.

  1. I inserted the Table block in the playground and rerun accessibility tests to reproduce one of the issues discovered when upgrading Puppeteer earlier today:
    Screen Shot 2019-10-31 at 15 44 20

  2. We should also make it possible to load the source of the story for the playground:
    Screen Shot 2019-10-31 at 15 47 53
    It should be easy to fix.

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 31, 2019

It's possible to see the source with 37aee14:

Screen Shot 2019-10-31 at 16 05 07

@epiqueras

This comment has been minimized.

Copy link
Contributor

epiqueras commented Oct 31, 2019

@gziolo We can add a netlify.toml so that this can be changed with code instead of having to ping me to log in to the dashboard.

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Oct 31, 2019

@gziolo We can add a netlify.toml so that this can be changed with code instead of having to ping me to log in to the dashboard.

It would be great 馃憤 It isn't a blocker for this PR but definitely nice to have as a quick follow-up.

@gziolo gziolo force-pushed the udpdate/storybook-playground branch from 37aee14 to 2a40755 Nov 4, 2019
@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Nov 4, 2019

@mkaz, I moved and updated README file added to the storybook folder in #18245 with 2a40755.

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Nov 5, 2019

@ItsJonQ - do you think it鈥檚 fine to move forward with this proposal? Did I miss anything?

@ItsJonQ

This comment has been minimized.

Copy link
Contributor

ItsJonQ commented Nov 7, 2019

do you think it鈥檚 fine to move forward with this proposal

@gziolo Yes! I think this is fantastic ! Thank you so much for doing this 馃檹

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Nov 8, 2019

@youknowriad do you have any concerns? I plan to merge it on Monday.

Copy link
Contributor

youknowriad left a comment

I love this, I think we can probably get rid of the "Gutenberg Playground" header and consolidate all components under the "Components" section (Modal, FontSizePicker)

enforce: 'pre',
},
{
test: /\.scss$/,

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 12, 2019

Contributor

In theory we could avoid this by loading the produced CSS like we do for the components. I feel we should consolidate on one way of loading styles in Storybook.

This comment has been minimized.

Copy link
@gziolo

gziolo Nov 12, 2019

Author Member

@ItsJonQ - I guess this is a good follow-up to decide how we tackle CSS in Storybook.

};

export const _default = () => {
registerCoreBlocks();

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 12, 2019

Contributor

I guess it's probably better to wrap in useEffect( , []) to only run on mount?

This comment has been minimized.

Copy link
@gziolo

gziolo Nov 12, 2019

Author Member

The fun part is that this exported function can't take hooks as it isn't a component but some special method that wraps stories. Anyway, we probably move it to App and define as an effect there, I will give it a spin 馃憤

@gziolo

This comment has been minimized.

Copy link
Member Author

gziolo commented Nov 12, 2019

I have this locally, commit coming up soon:
Screen Shot 2019-11-12 at 11 18 06

I managed to address all comments 馃帀

@gziolo gziolo force-pushed the udpdate/storybook-playground branch from 2a40755 to 74148c6 Nov 12, 2019
@gziolo gziolo merged commit e137ea9 into master Nov 12, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@gziolo gziolo deleted the udpdate/storybook-playground branch Nov 12, 2019
@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Nov 12, 2019

Thanks @gziolo

CreativeDive added a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
鈥18191)

* Storybook: Try to integrate the playground into Storybook

* Load Noto font in storybook via preview-head.html

* Add scss support for Storybook!

* Remove unused parcel-bundler

* Update mentions of the playground with Storybook

* Fix issues found during self-review

* Add back playground:* aliasing storybook:* for the time being

* Add help info for old commands

* Ensure that source for the playground can be presented in Storybook

* Move and update the README document added for Storybook

* Resolve all issues raised in the review
@ktmn

This comment has been minimized.

Copy link

ktmn commented Nov 14, 2019

Unrelated to this PR, but any chance anyone would know what dependencies I need for the Gutenberg editor in a different context?

I'm trying to display it on a custom admin page, rendering the sameish thing as this story:

<SlotFillProvider>
    <DropZoneProvider>
        <BlockEditorProvider>
            <WritingFlow>
                <ObserveTyping>
                    <BlockList />
                </ObserveTyping>
            </WritingFlow>
        </BlockEditorProvider>
    </DropZoneProvider>
</SlotFillProvider>

The editor works, but the Image block does not allow to upload media (even if I run wp_enqueue_media() before enqueing the editor script) and Classic editor block doesn't work because window.wpEditorL10n.tinymce and window.tinymce are undefined.
Which dependencies do I need to add for the editor to work fully?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.