Skip to content

Storybook and sketchbook build#12

Open
Mxchaeltrxn wants to merge 9 commits intomainfrom
storybookAndSketchbookBuild
Open

Storybook and sketchbook build#12
Mxchaeltrxn wants to merge 9 commits intomainfrom
storybookAndSketchbookBuild

Conversation

@Mxchaeltrxn
Copy link
Member

@Mxchaeltrxn Mxchaeltrxn commented Aug 24, 2021

Three changes made:

  1. Built the sketchbook config with webpack instead of esbuild, enabling styled-jsx. Styles are now applied (see the brandmark below).
    image

  2. Fixed styles on Github icon. in the header (vertically centered it).

  3. Enable styles in Storybook by 'enabling' styled-jsx again via webpack (Github social link is now together, rather than apart like before).
    image

It'd probably be nice to deploy this as well on Vercel with the domain demo-build.sketchbookjs.com for easier access but I haven't done that yet.

@Mxchaeltrxn Mxchaeltrxn requested a review from haydn August 24, 2021 09:19
@vercel
Copy link

vercel bot commented Aug 24, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

website – ./

🔍 Inspect: https://vercel.com/sketchbook-js/website/Cf4afaf1UWftz5K8BToKFAoLW8N7
✅ Preview: https://website-git-storybookandsketchbookbuild-sketchbook-js.vercel.app

website-storybook – ./

🔍 Inspect: https://vercel.com/sketchbook-js/website-storybook/6DwyJLfCMwEygEHD5F4pwap5w8uv
✅ Preview: https://website-storybook-git-storybookandsketchbookbuild-sketchbook-js.vercel.app

Copy link
Member

@haydn haydn left a comment

Choose a reason for hiding this comment

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

@Mxchaeltrxn Nice! You got both of them working. Sorry for the slow review.

I've added a bunch of comments cause there's a lot a little things here and there, but I've also fixed them all up — I'll push up the commits right after this.

@Mxchaeltrxn
Copy link
Member Author

@haydn Hey thanks for the review.

You haven't yet merged so I'm thinking the remaining part of this PR is to deploy to the URL

website-demo.sketchbookjs.com

Because

demo.sketchbookjs.com

Is already taken by the sketchbook repository.

Just checking since you didn't mention any other tasks in your comments.

@haydn
Copy link
Member

haydn commented Sep 6, 2021

@Mxchaeltrxn Hello hello! Again, apologies for the slow response!

Yeah, let's setup a project in Vercel called website-sketchbook (to match the website-storybook project that already exists). Yep, let's use website-demo.sketchbookjs.com for now, it's super easy to change later if we want.

There are 2 changes I think we should make as well:

  1. Currently, the global CSS is duplicated between the pages/index.js, .storybook/preview.js and sketchbook/config.jsx files. It'd be better for those styles to be defined once in their own component and imported into those different locations. For the Sketchbook config it'll probably need to use styled-jsx server-side rendering.
  2. We can probably simplify the Storybook setup by removing the @storybook/addon-essentials addon. That means we won't have all actions, controls etc. For our purposes that's probably fine because we just want to demonstrate the components getting loaded into Stroybook. With that addon removed the stories definitions can be massively simplified so they don't have all that confusing args and template config.

If you'd like some inspiration, I've recently setup Storybook in this project I've been putting together for ColonyDB (it's a bit different because it's still using a few addons):

https://github.com/colonydb/anthill

I'm planning to add Sketchbook to that project soon too! 😄

@haydn
Copy link
Member

haydn commented Sep 6, 2021

Actually… after thinking about it some more, I reckon you'll be able to get that global CSS into sketchbook/config.jsx using just ReactDOM.render(). Pretty sure something along these lines would work:

const globalStylesDiv = doc.createElement("div");
doc.body.appendChild(globalStylesDiv);
ReactDOM.render(<GlobalCSSComponentThingy />, globalStylesDiv);

@Mxchaeltrxn
Copy link
Member Author

I'm still working on this so don't review it yet.

@Mxchaeltrxn
Copy link
Member Author

Mxchaeltrxn commented Sep 19, 2021

@haydn

Changes made

  • Put the duplicated CSS in one spot, much like you did in anthill.
  • Removed the storybook essentials addon and simplified the stories code.

It seems like there isn't an easy way for me to create a vercel project and test a production build before merging this to main branch so I'll just wait for that before I do that.

Question

On a semi-related note, should I be running yarn sketchbook:build and then trying to open ./sketchbook-build/index.html and seeing if that loads Sketchbook to test the production build? I've been trying to do that locally but nothing seems to show up (I get a white screen), which makes me think that either something is wrong with the build or I'm not doing the right thing to load the build.

Also don't worry at all about your reply speed—I'm just glad you still review what I do!

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.

2 participants