From be93d7dde91f5a876b286591cc822f3437166a52 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Sat, 4 Jan 2020 14:14:56 +0800 Subject: [PATCH 1/5] Improve error message and add test case --- lib/ui/src/containers/nav.js | 9 ++++----- lib/ui/src/containers/nav.test.js | 29 +++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/ui/src/containers/nav.js b/lib/ui/src/containers/nav.js index 9bc5dc0533e8..d235ea77f450 100755 --- a/lib/ui/src/containers/nav.js +++ b/lib/ui/src/containers/nav.js @@ -131,18 +131,17 @@ export const collapseAllStories = stories => { const component = { ...rest, id: leafId, isLeaf: true, isComponent: true }; componentIdToLeafId[id] = leafId; - if ( - (isComponent && nonLeafChildren.length > 0) || - (!isComponent && nonLeafChildren.length === 0) - ) { + if (!isComponent && nonLeafChildren.length === 0) { throw new Error( `Unexpected '${item.id}': ${JSON.stringify({ isComponent, nonLeafChildren })}` ); } if (nonLeafChildren.length > 0) { + const nodeType = isComponent ? 'Component' : 'Folder'; + logger.error( - `Node ${item.id} contains non-leaf nodes that are getting removed: ${nonLeafChildren}!` + `${nodeType} '${item.id}' contains non-leaf nodes that are removed: '${nonLeafChildren}'!` ); } diff --git a/lib/ui/src/containers/nav.test.js b/lib/ui/src/containers/nav.test.js index 91ece2ac1feb..ac4bb24799c5 100644 --- a/lib/ui/src/containers/nav.test.js +++ b/lib/ui/src/containers/nav.test.js @@ -58,12 +58,37 @@ describe('collapse all stories', () => { ...stories, a1: { ...a1, ...docsOnly }, }; - const filtered = collapseAllStories(hasDocsOnly); - expect(filtered.a1).toEqual({ + const collapsed = collapseAllStories(hasDocsOnly); + expect(collapsed.a1).toEqual({ id: 'a1', isComponent: true, isLeaf: true, parent: 'root', }); }); + // eslint-disable-next-line jest/no-disabled-tests + it.skip('collapses mixtures of leaf and non-leaf children', () => { + const mixedRoot = { id: 'mixedRoot', parent: false, children: ['a', 'b1'] }; + const mixed = { mixedRoot, a, a1, b1 }; + const collapsed = collapseAllStories(mixed); + expect(collapsed).toEqual({ + a1: { + id: 'a1', + isComponent: true, + isLeaf: true, + parent: 'mixedRoot', + }, + b1: { + id: 'b1', + isComponent: true, + isLeaf: true, + parent: 'mixedRoot', + }, + mixed: { + children: ['a1', 'b1'], + id: 'mixedRoot', + parent: false, + }, + }); + }); }); From 09164b9c11ed2c4b67d29a7cbde35f7f85874659 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 6 Jan 2020 17:16:52 +0800 Subject: [PATCH 2/5] Nav UI: Don't collapse a node when it contains non-leaf children --- .../mixed-leaves-component.stories.js | 13 ++++++++++++ .../addon-docs/mixed-leaves-folder.stories.js | 12 +++++++++++ lib/ui/src/containers/nav.js | 21 ++++++++++--------- lib/ui/src/containers/nav.test.js | 16 +++++++------- 4 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js create mode 100644 examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js diff --git a/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js b/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js new file mode 100644 index 000000000000..4b8db33328e4 --- /dev/null +++ b/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js @@ -0,0 +1,13 @@ +// This example exists solely to demonstrate nav hierarchy +// in --docs mode when a folder contains both a component and +// individual stories +// +// See also ./mixed-leaves-folder.stories.js + +export default { + title: 'Addons/Docs/MixedLeaves/Component', + parameters: { chromatic: { disable: true } }, +}; + +export const B = () => 'b'; +export const C = () => 'c'; diff --git a/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js b/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js new file mode 100644 index 000000000000..118dfe4bb694 --- /dev/null +++ b/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js @@ -0,0 +1,12 @@ +// This example exists solely to demonstrate nav hierarchy +// in --docs mode when a folder contains both a component and +// individual stories +// +// See also ./mixed-leaves-component.stories.js + +export default { + title: 'Addons/Docs/MixedLeaves', + parameters: { chromatic: { disable: true } }, +}; + +export const A = () => 'a'; diff --git a/lib/ui/src/containers/nav.js b/lib/ui/src/containers/nav.js index d235ea77f450..254737fc4fc8 100755 --- a/lib/ui/src/containers/nav.js +++ b/lib/ui/src/containers/nav.js @@ -114,11 +114,17 @@ export const collapseAllStories = stories => { const componentIdToLeafId = {}; // 1) remove all leaves - const leavesRemoved = Object.values(stories).filter(item => !item.isLeaf); + const leavesRemoved = Object.values(stories).filter( + item => !(item.isLeaf && stories[item.parent].isComponent) + ); // 2) make all components leaves and rewrite their ID's to the first leaf child const componentsFlattened = leavesRemoved.map(item => { const { id, isComponent, children, ...rest } = item; + if (!isComponent) { + return item; + } + const nonLeafChildren = []; const leafChildren = []; children.forEach(child => (stories[child].isLeaf ? leafChildren : nonLeafChildren).push(child)); @@ -131,20 +137,15 @@ export const collapseAllStories = stories => { const component = { ...rest, id: leafId, isLeaf: true, isComponent: true }; componentIdToLeafId[id] = leafId; - if (!isComponent && nonLeafChildren.length === 0) { + if ( + (!isComponent && nonLeafChildren.length === 0) || + (isComponent && nonLeafChildren.length !== 0) + ) { throw new Error( `Unexpected '${item.id}': ${JSON.stringify({ isComponent, nonLeafChildren })}` ); } - if (nonLeafChildren.length > 0) { - const nodeType = isComponent ? 'Component' : 'Folder'; - - logger.error( - `${nodeType} '${item.id}' contains non-leaf nodes that are removed: '${nonLeafChildren}'!` - ); - } - return component; }); diff --git a/lib/ui/src/containers/nav.test.js b/lib/ui/src/containers/nav.test.js index ac4bb24799c5..96ef56a634f2 100644 --- a/lib/ui/src/containers/nav.test.js +++ b/lib/ui/src/containers/nav.test.js @@ -66,27 +66,25 @@ describe('collapse all stories', () => { parent: 'root', }); }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip('collapses mixtures of leaf and non-leaf children', () => { - const mixedRoot = { id: 'mixedRoot', parent: false, children: ['a', 'b1'] }; - const mixed = { mixedRoot, a, a1, b1 }; + it('collapses mixtures of leaf and non-leaf children', () => { + const mixedRoot = { id: 'root', parent: false, children: ['a', 'b1'] }; + const mixed = { root: mixedRoot, a, a1, b1: { ...b1, parent: 'root' } }; const collapsed = collapseAllStories(mixed); expect(collapsed).toEqual({ a1: { id: 'a1', isComponent: true, isLeaf: true, - parent: 'mixedRoot', + parent: 'root', }, b1: { id: 'b1', - isComponent: true, isLeaf: true, - parent: 'mixedRoot', + parent: 'root', }, - mixed: { + root: { children: ['a1', 'b1'], - id: 'mixedRoot', + id: 'root', parent: false, }, }); From 20c1ef0b8570a285edd5ca1cb666afc57681b47b Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 6 Jan 2020 17:22:10 +0800 Subject: [PATCH 3/5] Fix deepscan issues --- lib/ui/src/containers/nav.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ui/src/containers/nav.js b/lib/ui/src/containers/nav.js index 254737fc4fc8..3ce37bdb04fb 100755 --- a/lib/ui/src/containers/nav.js +++ b/lib/ui/src/containers/nav.js @@ -121,6 +121,8 @@ export const collapseAllStories = stories => { // 2) make all components leaves and rewrite their ID's to the first leaf child const componentsFlattened = leavesRemoved.map(item => { const { id, isComponent, children, ...rest } = item; + + // this is a folder, so just leave it alone if (!isComponent) { return item; } @@ -137,10 +139,8 @@ export const collapseAllStories = stories => { const component = { ...rest, id: leafId, isLeaf: true, isComponent: true }; componentIdToLeafId[id] = leafId; - if ( - (!isComponent && nonLeafChildren.length === 0) || - (isComponent && nonLeafChildren.length !== 0) - ) { + // this is a component, so it should not have any non-leaf children + if (nonLeafChildren.length !== 0) { throw new Error( `Unexpected '${item.id}': ${JSON.stringify({ isComponent, nonLeafChildren })}` ); From a61da5864cacefacff97c3b142f98172523acb99 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 6 Jan 2020 17:24:59 +0800 Subject: [PATCH 4/5] Fix deepscan --- lib/ui/src/containers/nav.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/src/containers/nav.js b/lib/ui/src/containers/nav.js index 3ce37bdb04fb..0989c6ac84d0 100755 --- a/lib/ui/src/containers/nav.js +++ b/lib/ui/src/containers/nav.js @@ -4,7 +4,6 @@ import memoize from 'memoizerific'; import { Badge } from '@storybook/components'; import { Consumer } from '@storybook/api'; -import { logger } from '@storybook/client-logger'; import { shortcutToHumanString } from '../libs/shortcut'; From 9f96a457b8c27425a6778e37c37c8435531b5401 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Mon, 6 Jan 2020 20:40:26 +0800 Subject: [PATCH 5/5] Tweak stories --- .../stories/addon-docs/mixed-leaves-component.stories.js | 2 +- .../stories/addon-docs/mixed-leaves-folder.stories.js | 2 +- .../stories/core/named-export-order.stories.js | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js b/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js index 4b8db33328e4..3d6a49874f1c 100644 --- a/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js +++ b/examples/official-storybook/stories/addon-docs/mixed-leaves-component.stories.js @@ -5,7 +5,7 @@ // See also ./mixed-leaves-folder.stories.js export default { - title: 'Addons/Docs/MixedLeaves/Component', + title: 'Addons/Docs/Mixed Leaves/Component', parameters: { chromatic: { disable: true } }, }; diff --git a/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js b/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js index 118dfe4bb694..2f9216606fe9 100644 --- a/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js +++ b/examples/official-storybook/stories/addon-docs/mixed-leaves-folder.stories.js @@ -5,7 +5,7 @@ // See also ./mixed-leaves-component.stories.js export default { - title: 'Addons/Docs/MixedLeaves', + title: 'Addons/Docs/Mixed Leaves', parameters: { chromatic: { disable: true } }, }; diff --git a/examples/official-storybook/stories/core/named-export-order.stories.js b/examples/official-storybook/stories/core/named-export-order.stories.js index 34c54ca0e2e8..03fda7eb5bcf 100644 --- a/examples/official-storybook/stories/core/named-export-order.stories.js +++ b/examples/official-storybook/stories/core/named-export-order.stories.js @@ -1,7 +1,6 @@ -import React from 'react'; - export default { title: 'Core/Named Export Order', + parameters: { chromatic: { disable: true } }, }; export const Story1 = () => 'story1';