guppy icon indicating copy to clipboard operation
guppy copied to clipboard

Experiment with Docz

Open joshwcomeau opened this issue 7 years ago • 33 comments

Docz is an alternative to Storybook. It uses MDX, which is a cross between Markdown and JSX, and it's really nice for this kind of thing.

I gave it a quick try with our Button component, and I quite like it: screen shot 2018-09-17 at 9 22 26 am

I uploaded it to Surge, play with it here: http://guppy-docz.surge.sh/src-components-button-stroke-button

Pros:

  • Shows the props in a really convenient way
  • Playground component is really nice, love that it shows JSX and HTML
  • Much easier configuration, it took 2 minutes to set up
  • Less code to write when adding new stories/docs
  • Looks really nice, I prefer the UX of it compared to Storybook

Cons:

  • Newer project, less mature. Might disappear in a month
  • Doesn't support using some standard ESNext stuff like spreading props or using property initializer syntax. It works if you use it in your components, but not if you use it in the .mdx.
  • MDX doesn't yet support Prettier. They've already merged a PR for it, but it isn't released yet.
  • Some things are more awkward, since you can't just write JS anywhere, it has to be within JSX.
  • We already have invested some time in Storybook, not all of it will be reusable, so it's a bit extra upfront work

Really curious what y'all think. I think I'm leaning towards Docz, but it's not a strong feeling. I also expect Docz might improve a lot over the next few months, so maybe we can revisit this later?

joshwcomeau avatar Sep 17 '18 13:09 joshwcomeau

MDX doesn't yet support Prettier. They've already merged a PR for it, but it isn't released yet.

You can always install prettier/prettier to get the bleeding-edge versión from GitHub.

j-f1 avatar Sep 17 '18 15:09 j-f1

ok, I think it's worth switching to Docz. It's easier to write stories, and the end result is nicer / more usable, IMO.

Will add a new issue

joshwcomeau avatar Sep 23 '18 13:09 joshwcomeau

Heads-up, if anyone's interested: I'm gonna convert all of our existing stories to docz in the next week or so. I have some good ideas for how this could become our style-guide, making it easier for folks to contribute to the UI :D

joshwcomeau avatar Oct 01 '18 13:10 joshwcomeau

This branch is now properly deployed to Netlify. It looks like the homepage is not defined — should we put something there?

j-f1 avatar Oct 01 '18 15:10 j-f1

It looks like the homepage is not defined — should we put something there?

Yep, I'm on it.

I'm excited to treat Docz as more of a "style guide". It should help developers understand how to contribute to the UI, not just be a component library that shows what props are available (although it should do that too).

So the homepage will include some info about what it's for, and links to common UI pieces.

joshwcomeau avatar Oct 02 '18 14:10 joshwcomeau

@j-f1 it looks like client-side nav links aren't respected (the link you provided is dead, since netlify doesn't know to redirect that URL to the homepage). Is there a way to set up Netlify to handle client-side navigation?

joshwcomeau avatar Oct 02 '18 14:10 joshwcomeau

@joshwcomeau Is it handled by a router? Perhaps this will help.

melanieseltzer avatar Oct 02 '18 14:10 melanieseltzer

All fixed @joshwcomeau! I used a similar technique to the one @melanieseltzer suggested, but I put it in the netlify.toml file instead.

j-f1 avatar Oct 02 '18 14:10 j-f1

@joshwcomeau I've modified the structure of the docz a bit and added an index route. Nothing pushed yet - it's just locally. If you like I can push it to docz branch.

The content of index page Guppy Style Guide in my WIP is just to have some content and should be improved.

grafik

I really like the new docs from react-spring with docz. Maybe we can do it similar but I think we need more structure like in my screenshot above.

I'd also like to help to port the storybook stories to docz. What do you think can we merge the basic setup from here to master and create an new issue for tracking the work?

AWolf81 avatar Nov 08 '18 23:11 AWolf81

Prettier supports MDX now 🎉

j-f1 avatar Nov 09 '18 14:11 j-f1

I think it's safe to say that docz and MDX are here to stay. This PR will need to be rebased and brought up to speed with master, then all remaining stories will need to be converted to docz. I'll handle the rebase tonight, then anyone who'd like to champion the rewrite feel free to chime in below!

EDIT: There have been some big changes to docz in the last few weeks, and it looks like a few things are required to move forward with this PR:

  • update react/react-dom to support Hooks API (>16.8.0)
  • update docz/docz-theme-default to the next release
  • regenerate yarn.lock (ref)

EDIT 2: Currently, there's no way to have 3rd+ layer nested menu items, so we'll go with a flatter structure for now and track progress here.

EDIT 3: The activeClassName and partiallyActive DOM errors on every page are being tracked here. The 1.0 release is only a few days old at this point so there's still a few bugs to be ironed out, it seems.

EDIT 4: Both Flow and styled-components currently require some extra config to work correctly with the <Props /> component, which is a core feature of docz. If this workflow isn't improved in the v1 release then docz may not in fact be a good fit for this app. There's also a soft dependency on styled-components@^4.0.0, which we're not currently using (see #213 to track progress). Let's check back in a couple weeks to determine if the future of Guppy+docz is promising or not.

superhawk610 avatar Mar 27 '19 00:03 superhawk610

Codecov Report

Merging #254 into master will decrease coverage by 0.74%. The diff coverage is 2.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
- Coverage   58.83%   58.08%   -0.75%     
==========================================
  Files         158      163       +5     
  Lines        3357     3400      +43     
  Branches      467      471       +4     
==========================================
  Hits         1975     1975              
- Misses       1179     1218      +39     
- Partials      203      207       +4
Impacted Files Coverage Δ
...CreateNewProjectWizard/Gatsby/SelectStarterList.js 100% <ø> (ø) :arrow_up:
src/components/Heading/Heading.js 75% <ø> (ø) :arrow_up:
src/docz/components/ProgressManager/index.js 0% <0%> (ø)
src/components/OnlineChecker/OnlineChecker.js 0% <0%> (ø) :arrow_up:
src/docz/components/Toggleable/index.js 0% <0%> (ø)
src/docz/components/Toggleable/Toggleable.js 0% <0%> (ø)
src/components/LoadingScreen/LoadingScreen.js 0% <0%> (ø) :arrow_up:
...docz/components/ProgressManager/ProgressManager.js 0% <0%> (ø)
src/docz/colors/ColorList.js 0% <0%> (ø)
src/utils.js 39.04% <100%> (ø) :arrow_up:
... and 5 more

codecov[bot] avatar Mar 27 '19 01:03 codecov[bot]

@superhawk610 I've been using Docz for one of my side projects and it's been working well. It seems to be more stable now at 1.1.0, so I went ahead and updated this branch :)

Item 3 and 4 from your list seem resolved. Unsure about 2, haven't looked into it.

We are experiencing https://github.com/pedronauck/docz/issues/838, tracking that thread. Doesn't seem to be causing error, just throwing up a warning.

We experience https://github.com/pedronauck/docz/issues/793 but adding react-hot-loader as an explicit devDep to update it fixes it for now.

What do you think?

[Edit] Not sure why Flow is failing in CircleCI...

[Edit2] Huh CI is fixed now 🤷‍♀

melanieseltzer avatar May 04 '19 20:05 melanieseltzer

I'm OK with pinning peer dependencies so the react-hot-loader thing doesn't bother me (I wish package.json supported inline comments so we could note why we're pinning it but that's another discussion entirely).

As for 2, I'm personally a fan of nested menu structures, eg

Components
  Buttons
    OutlineButton
    FillButton
  Animated
    LoadingSpinner
    GuppyLoader
Styles
  General
  All

but we can definitely get by with a flat structure, so that's not really a blocker.

3 and 4 were the largest blocks IMO so now that they're fixed I think this should definitely become a priority PR. What do you think is left to do before this can be merged?

superhawk610 avatar May 06 '19 13:05 superhawk610

I don't think there's a crazy amount to do, I'll probably try tackle some of this list this week or weekend :)

  • [x] Index page route for Docz. @AWolf81 I see you had done some work earlier in this PR thread surrounding the index route, but I don't see that in this branch. Do you still have it?
  • [x] Convert the lingering Storybook components:
    • [x] ButtonWithIcon
    • [x] CircularIcon
    • [x] Mount after (is this one necessary or is Docz meant to be more of a visual style guide...?)
    • [x] Planet
    • [x] ProgressBar
    • [x] ProjectIconSelection
  • [ ] Port running list from https://github.com/joshwcomeau/guppy/issues/131 to new issue that's Docz focus (or change that issue to be Docz focused)
  • [ ] Remove storybook related configs and files

Adding new mdx files can come in a new PR, we can focus on getting Docz to where Storybook is and then expand later.

Will the URL still be https://guppy.netlify.com? I see the netlify.toml in this branch with the updated build command, but will it only build once this is merged to master?

melanieseltzer avatar May 08 '19 04:05 melanieseltzer

@melanieseltzer Sorry, I can't find the stuff I created in November - seems like I didn't commit that. No local docz branch & nothing about it in reflog.

For deploying: It's possible to do a branch deploy & it seems that it is enabled as deploying task is started but it fails with a message like build-docz not found. See my inline comment on how to fix it - just the wrong name in netlify.toml.

AWolf81 avatar May 08 '19 05:05 AWolf81

@AWolf81 No worries, I'll use the stuff from your screenshot and build on that 👍 Thanks for spotting the wrong command! It's up now with a deploy preview.

melanieseltzer avatar May 08 '19 06:05 melanieseltzer

@superhawk610 @AWolf81 Alright, lingering Storybook stories have been converted, and cleaned things up a bit (also added the logo!). Had to add Docz-specific components (src/docz/components) because those logics were in the storybook js files directly, and mdx can only import components.

Is there anything you think we could improve upon before a merge?

melanieseltzer avatar May 09 '19 06:05 melanieseltzer

@melanieseltzer Looks great. Well done. 👍

I'll do a review later today.

For the styling I have one small point - I think it would look better if the Guppy logo is centered in the sidebar. What do you think?

grafik

In the docz I've noticed that in ProjectIconSelection selecting an icon is not working. It would be great if the selection is working.

It would be also great if in the index page there would be a short explanation how to add new Docz stories so new devs are getting an idea where and how to add them. Just adding an small example .mdx and that the store will live in the same directory as component will be a good starting point. Also a link to docz getting started docs would be nice.

Any ideas why Circle CI is failing? I've tried to re-build but Flow is failing. I thinks it's a CircleCI issue and not with our code.

AWolf81 avatar May 09 '19 06:05 AWolf81

Agreed, centered logo looks better, updated it + expanded the getting started info per your suggestions 👍 Will punt on selecting an icon on click for ProjectIconSelection, was also thinking it could be good to show.

I'm also confused why Flow is failing in CircleCI, it looks like it's also happening in #374 as well. No problems locally so this is confusing...

melanieseltzer avatar May 09 '19 07:05 melanieseltzer

Unfortunately, I don't have any suggestions as to how to resolve the Flow CI issue, but I did experience it quite a bit when actively working on this project (especially on Windows). I could never reliably reproduce it but it cropped up every other day or so and nothing consistently solved it (nuking node_modules, using global/local flow-bin, upgrading/downgrading, etc). That's one of the major reasons I switched to TS exclusively, but obviously that's not an option for this project.

Here is a thread dating back almost 4 years showing that this issue continues to crop up. The common response is "mismatched global local versions" but I don't know how that could be the case in CI. Maybe we need to bump the flow-bin version?

superhawk610 avatar May 09 '19 13:05 superhawk610

@superhawk610 I'm running out of ideas - I've tested some of the comments from this issue but with-out success.

I've tried:

  • Updated flow version to 0.98.1
  • Rebuild with-out cache
  • Remove [include] section from flow config
  • Run flow check instead of flow - different error message but I'm not sure what to do with it. I'll search for it later.

On Windows it's also really slow for me and VS code liniting is disabled as two running servers are a problem for flow.

I think converting to Typescript is probably a pretty heavy task as we would have to modify many files - not sure if it's worth it. But would be an idea if we can't fix the flow issue.

[Edit]: I think Netlify is failing because I've updated the package.json in Github. I'll check yarn.lock file later - maybe there is a problem.

AWolf81 avatar May 09 '19 14:05 AWolf81

@AWolf81 perhaps this is the solution?

That thread was opened because of the exact errors we're now getting on CircleCI.

superhawk610 avatar May 09 '19 14:05 superhawk610

@superhawk610 thanks for pointing to that issue.

I think settings the max workers fixed it - not sure why. Next we need to check why Flow is failing - is it because of the previous flow check addition or is it a type error?

AWolf81 avatar May 09 '19 15:05 AWolf81

@AWolf81 looks to be a type error, I'll see if I can patch it.

EDIT: Alright, that got it. What should we do about the coverage? I wouldn't imagine Docz stuff should really be considered in coverage.

superhawk610 avatar May 09 '19 17:05 superhawk610

Codecov Report

Merging #254 into master will decrease coverage by 0.89%. The diff coverage is 1.81%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #254     +/-   ##
=========================================
- Coverage   58.83%   57.93%   -0.9%     
=========================================
  Files         158      165      +7     
  Lines        3357     3409     +52     
  Branches      467      471      +4     
=========================================
  Hits         1975     1975             
- Misses       1179     1227     +48     
- Partials      203      207      +4
Impacted Files Coverage Δ
...CreateNewProjectWizard/Gatsby/SelectStarterList.js 100% <ø> (ø) :arrow_up:
src/components/Heading/Heading.js 75% <ø> (ø) :arrow_up:
src/docz/components/ProgressManager/index.js 0% <0%> (ø)
src/components/OnlineChecker/OnlineChecker.js 0% <0%> (ø) :arrow_up:
src/docz/components/Toggleable/index.js 0% <0%> (ø)
src/docz/components/Toggleable/Toggleable.js 0% <0%> (ø)
src/components/LoadingScreen/LoadingScreen.js 0% <0%> (ø) :arrow_up:
src/docz/components/IconSelector/IconSelector.js 0% <0%> (ø)
...docz/components/ProgressManager/ProgressManager.js 0% <0%> (ø)
src/docz/colors/ColorList.js 0% <0%> (ø)
... and 9 more

codecov[bot] avatar May 09 '19 17:05 codecov[bot]

@superhawk610 yes, we can exlclude it from coverage by updating this line in package.json.

What do you think should we revert the change with flow check? For me flow is not starting locally with it. But I'm not sure what's the difference between check/server & status. Even after reading this SO.

AWolf81 avatar May 09 '19 18:05 AWolf81

I believe flow check is the correct command to use for CI since it's self-contained - it starts and stops discretely. I can confirm flow check works locally for me on Mac, but it's always worked better on Mac than Windows.

superhawk610 avatar May 09 '19 18:05 superhawk610

@AWolf81 I got the project icon selection working, let me know what you think?

Btw, flow check works for me locally on Mac but it took a while to finish checking.

melanieseltzer avatar May 11 '19 23:05 melanieseltzer

I wanted to check Docz but my build with yarn docz:dev is not working.

On first try it complains about missing @mdx-js/react. Once it's installed it fails with the following error in browser: grafik

That's also mentioned in this issue but the React dependency seems correct. I also tried to reinstall node_modules & recreating yarn-lock file.

But still not working. My setup:

  • Node v10.16.0
  • Windows 10
  • Docz 1.1.0 (also updating Docz to latest 1.x version wasn't working)

I also tried Docz@next but that failed with a build error ./src/constants not found and I'm not sure how to fix it.

AWolf81 avatar Oct 12 '19 22:10 AWolf81