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: Support docs-only MDX && update docs mode nav UI #7302

Closed
wants to merge 5 commits into from

Conversation

shilman
Copy link
Member

@shilman shilman commented Jul 4, 2019

Issue: #6644

What I did

  • Updated MDX compiler to gen a "docsOnly" story when MDX has no stories
  • Updated UI to show "docsOnly" stories in both canvas & docs modes
  • Updated DOCS_MODE UI to show only component-level nodes
  • Added example to official-storybook
  • Documented future enhancements in Addon-docs: Navigation enhancements #7304

Here's how a docs-only node shows up in the UI:

Storybook

Here's how the tree looks in --docs mode:

Storybook

How to test

cd examples/official-storybook
yarn storybook
yarn storybook --docs

@vercel
Copy link

vercel bot commented Jul 4, 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-6644-docs-only-stories.storybook.now.sh

@shilman shilman changed the title Addon-docs: Support docs-only MDX && update docs mode UI Addon-docs: Support docs-only MDX && update docs mode nav UI Jul 4, 2019
@shilman shilman added this to the 5.2.0 milestone Jul 4, 2019
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.

I think there is a bug here

const { id, isLeaf, parent } = story;
if (isLeaf) {
if (!filtered[parent].isLeaf) {
filtered[parent].isLeaf = true;
Copy link
Member

Choose a reason for hiding this comment

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

This logic seems wrong. It is possible to have both leaves and non leaves as children. For instance

GroupA
|-- GroupB
|      \-- Story1
\-- Story 2

I think there are some more complex examples in the code base, eg. in https://github.com/storybookjs/storybook/blob/next/lib/ui/src/components/sidebar/treeview/treeview.mockdata.js#L1.

I think to be correct, you'd need to delete story from parent.children, then set it to isLeaf if it is empty

Copy link
Member

@tmeasday tmeasday Jul 4, 2019

Choose a reason for hiding this comment

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

Can we just do this (without clone) in a fairly simple way?:

return stories.filter(s => !s.isLeaf).map(({ children, ...story }) => {
  const newChildren = children.filter(s => !s.isLeaf);
  return newChildren.length === 0 ? 
    {...story, isLeaf: true } : 
    {...story, children: newChildren};
});

}
};

export const removeDocsOnlyStories = stories => {
Copy link
Member

Choose a reason for hiding this comment

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

What is this for? It only seems to be used in tests?

const { id, isLeaf, parent, parameters } = story;
if (isLeaf && parameters && parameters.docsOnly) {
if (!filtered[parent].isLeaf) {
filtered[parent].isLeaf = true;
Copy link
Member

Choose a reason for hiding this comment

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

In theory this has the same problem as removeLeafStories.

Also you could probably do a no-cloneDeep version very similar to below

Copy link
Member

Choose a reason for hiding this comment

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

In fact a removeLeaves(stories, predicate?) function would make sense and remove the need for the duplicate logic.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2019

Fails
🚫 PR is marked with "do not merge" label.

Generated by 🚫 dangerJS against 1a4c969

@shilman shilman closed this Jul 5, 2019
@stof stof deleted the 6644-docs-only-stories branch October 4, 2019 09:58
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.

2 participants