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

Addon-docs: DocsPage and doc blocks #7119

Merged
merged 17 commits into from Jun 21, 2019
Merged

Addon-docs: DocsPage and doc blocks #7119

merged 17 commits into from Jun 21, 2019

Conversation

shilman
Copy link
Member

@shilman shilman commented Jun 18, 2019

Issue: #7101

What I did

This PR adds DocsPage and all the doc blocks. It's a big one, but hopefully the code is pretty simple-minded.

In summary:

  • lib/components/src/blocks/* - pure components that have their own stories in official-storybook
  • addons/docs/src/blocks/* - container components that do various tricks to get stuff out of the context/etc. to provide data to the pure components
  • hook up this code into examples through DocsPage which automatically generates pages for each "kind"

NOTES:

How to test

  • Inspect doc blocks stories in official storybook
  • Inspect "docs" tab in official storybook
  • Inspect "docs" tab in vue example
cd examples/official-storybook
yarn storybook

cd ../vue-kitchen-sink
yarn storybook --no-dll

@vercel
Copy link

vercel bot commented Jun 18, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-7101-doc-blocks.storybook.now.sh

@shilman shilman mentioned this pull request Jun 18, 2019
21 tasks
@vercel vercel bot temporarily deployed to staging June 18, 2019 08:57 Inactive
@tmeasday
Copy link
Member

@shilman can you ping me when this is ready to review (or else you could change the base temporarily)

@vercel vercel bot temporarily deployed to staging June 18, 2019 15:15 Inactive
@shilman
Copy link
Member Author

shilman commented Jun 18, 2019

@tmeasday This is ready to review now

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Hard to review all of this but it seems generally high quality. Anything in particular you wanted me to look at?

addons/docs/src/blocks/Description.tsx Outdated Show resolved Hide resolved
addons/docs/src/blocks/DocsContainer.tsx Show resolved Hide resolved
addons/docs/src/blocks/DocsContainer.tsx Show resolved Hide resolved
addons/docs/src/blocks/DocsContext.ts Show resolved Hide resolved
lib/components/src/blocks/IFrame.tsx Outdated Show resolved Hide resolved
@tmeasday
Copy link
Member

tmeasday commented Jun 19, 2019 via email

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM!

addons/docs/src/blocks/DocsContext.ts Show resolved Hide resolved
@ndelangen
Copy link
Member

It doesn't recover from errors:

Screenshot 2019-06-21 at 15 14 11

Reproduction path:

  • Navigate docs-mode
  • Navigate to an story that throws and error
  • Navigate to a functioning story

addons/docs/src/blocks/Story.tsx Outdated Show resolved Hide resolved
addons/docs/src/blocks/DocsPage.tsx Outdated Show resolved Hide resolved
expanded = true,
}) => (
<>
{expanded && <h3>{name}</h3>}
Copy link
Member

Choose a reason for hiding this comment

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

Should this be configurable at all?

why not just always show this?

Is creating a different docBlock for "showing all stories" not a better api?

configurability is the root of all evil

@@ -0,0 +1,136 @@
import React from 'react';

import { parseKind } from '@storybook/router';
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to rename this at some point, to something a little more descriptive of what is actually does.

addons/docs/src/blocks/Description.tsx Show resolved Hide resolved

const DocsPage: React.FunctionComponent<DocsPageProps> = ({ title, subtitle, children }) => (
<>
{title && <Title>{title}</Title>}
Copy link
Member

Choose a reason for hiding this comment

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

Don't we have shared typography components?

Copy link
Member

Choose a reason for hiding this comment

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

or should have them

@@ -0,0 +1,155 @@
import React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

reuse for markdown tables?

Copy link
Member

Choose a reason for hiding this comment

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

Can we set tables in a horizontal scroller container so mobile users can scroll to view the entire thing?

I think it currently makes up the entire viewport scrollable horizontally

<MyComponent boolProp scalarProp={1} complexProp={{ foo: 1, bar: '2' }}>
<SomeOtherComponent funcProp={(a) => a.id} />
</MyComponent>
`.trim();
Copy link
Member

Choose a reason for hiding this comment

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

these trims are causing the bad indenting, because the formatter expects the whitespace from template literals.

}));

export interface TypesetProps {
fontSizes: number[];
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't a font-face / font-family prop makes sense here?

@vercel vercel bot temporarily deployed to staging June 21, 2019 17:16 Inactive
@vercel vercel bot temporarily deployed to staging June 21, 2019 17:23 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants