From e3ac03cd68f6e36571d31b6e37bfb09ee17a3feb Mon Sep 17 00:00:00 2001
From: Filipp Riabchun <fryabchun@gmail.com>
Date: Fri, 4 Aug 2017 15:09:55 +0300
Subject: [PATCH] Always collapse an expanded kind without changing selected
 story (#1590)

* Always collapse an expanded kind without changing selected story

* Reduce duplication in stories_tree tests
---
 .../left_panel/stories_tree/index.js          |   2 +-
 .../left_panel/stories_tree/index.test.js     | 140 +++++++-----------
 2 files changed, 52 insertions(+), 90 deletions(-)

diff --git a/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.js b/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.js
index f4733814b74..254ab61b40e 100644
--- a/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.js
+++ b/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.js
@@ -68,7 +68,7 @@ class Stories extends React.Component {
   onToggle(node, toggled) {
     if (node.story) {
       this.fireOnKindAndStory(node.kind, node.story);
-    } else if (node.kind) {
+    } else if (node.kind && toggled) {
       this.fireOnKind(node.kind);
     }
 
diff --git a/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.test.js b/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.test.js
index ffcca452097..dcde01d0ca5 100644
--- a/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.test.js
+++ b/lib/ui/src/modules/ui/components/left_panel/stories_tree/index.test.js
@@ -4,12 +4,27 @@ import Stories from './index';
 import { createHierarchy } from '../../../libs/hierarchy';
 
 describe('manager.ui.components.left_panel.stories', () => {
+  const data = createHierarchy([
+    { kind: 'a', stories: ['a1', 'a2'] },
+    { kind: 'b', stories: ['b1', 'b2'] },
+  ]);
+  const dataWithoutSeparator = createHierarchy([
+    { kind: 'some.name.item1', stories: ['a1', 'a2'] },
+    { kind: 'another.space.20', stories: ['b1', 'b2'] },
+  ]);
+  const dataWithSeparator = createHierarchy(
+    [
+      { kind: 'some.name.item1', stories: ['a1', 'a2'] },
+      { kind: 'another.space.20', stories: ['b1', 'b2'] },
+    ],
+    '\\.'
+  );
+
   describe('render', () => {
     test('should render stories - empty', () => {
-      const data = createHierarchy([]);
       const wrap = shallow(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={createHierarchy([])}
           selectedKind={''}
           selectedStory={''}
           selectedHierarchy={[]}
@@ -22,36 +37,25 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should render stories', () => {
-      const data = createHierarchy([
-        { kind: 'a', stories: ['a1', 'a2'] },
-        { kind: '20', stories: ['b1', 'b2'] },
-      ]);
       const wrap = shallow(
         <Stories
           storiesHierarchy={data}
-          selectedKind="20"
+          selectedKind="b"
           selectedStory="b2"
-          selectedHierarchy={['20']}
+          selectedHierarchy={['b']}
         />
       );
 
       const output = wrap.html();
 
-      expect(output).toMatch(/20/);
+      expect(output).toMatch(/b/);
       expect(output).toMatch(/b2/);
     });
 
     test('should render stories with hierarchy - hierarchySeparator is defined', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = shallow(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={['another', 'space', '20']}
@@ -73,13 +77,9 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should render stories without hierarchy - hierarchySeparator is not defined', () => {
-      const data = createHierarchy([
-        { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-        { kind: 'another.space.20', stories: ['b1', 'b2'] },
-      ]);
       const wrap = shallow(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithoutSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={['another.space.20']}
@@ -97,16 +97,9 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should render stories with initially selected nodes according to the selectedHierarchy', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = shallow(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={['another', 'space', '20']}
@@ -123,16 +116,9 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should contain state with all selected nodes after clicking on the nodes', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={['another', 'space', '20']}
@@ -153,16 +139,9 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should recalculate selected nodes after selectedHierarchy changes', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={[]}
@@ -181,16 +160,9 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should add selected nodes to the state after selectedHierarchy changes with a new value', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={['another', 'space', '20']}
@@ -213,16 +185,9 @@ describe('manager.ui.components.left_panel.stories', () => {
 
     test('should not call setState when selectedHierarchy prop changes with the same value', () => {
       const selectedHierarchy = ['another', 'space', '20'];
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="another.space.20"
           selectedStory="b2"
           selectedHierarchy={selectedHierarchy}
@@ -239,11 +204,7 @@ describe('manager.ui.components.left_panel.stories', () => {
   });
 
   describe('events', () => {
-    test('should call the onSelectStory prop when a kind is clicked', () => {
-      const data = createHierarchy([
-        { kind: 'a', stories: ['a1', 'a2'] },
-        { kind: 'b', stories: ['b1', 'b2'] },
-      ]);
+    test('should call the onSelectStory prop when a collapsed kind is clicked', () => {
       const onSelectStory = jest.fn();
       const wrap = mount(
         <Stories
@@ -261,11 +222,28 @@ describe('manager.ui.components.left_panel.stories', () => {
       expect(onSelectStory).toHaveBeenCalledWith('a', null);
     });
 
+    test("shouldn't call the onSelectStory prop when an expanded kind is clicked", () => {
+      const onSelectStory = jest.fn();
+      const wrap = mount(
+        <Stories
+          storiesHierarchy={data}
+          selectedKind="b"
+          selectedStory="b2"
+          selectedHierarchy={['b']}
+          onSelectStory={onSelectStory}
+        />
+      );
+
+      const kind = wrap.find('a').filterWhere(el => el.text() === 'a').last();
+      kind.simulate('click');
+
+      onSelectStory.mockClear();
+      kind.simulate('click');
+
+      expect(onSelectStory).not.toHaveBeenCalled();
+    });
+
     test('should call the onSelectStory prop when a story is clicked', () => {
-      const data = createHierarchy([
-        { kind: 'a', stories: ['a1', 'a2'] },
-        { kind: 'b', stories: ['b1', 'b2'] },
-      ]);
       const onSelectStory = jest.fn();
       const wrap = mount(
         <Stories
@@ -284,18 +262,10 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should call the onSelectStory prop when a story is clicked - hierarchySeparator is defined', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
-
       const onSelectStory = jest.fn();
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="some.name.item1"
           selectedStory="a2"
           selectedHierarchy={['some', 'name', 'item1']}
@@ -315,18 +285,10 @@ describe('manager.ui.components.left_panel.stories', () => {
     });
 
     test('should call the onSelectStory prop when a story is selected with enter key', () => {
-      const data = createHierarchy(
-        [
-          { kind: 'some.name.item1', stories: ['a1', 'a2'] },
-          { kind: 'another.space.20', stories: ['b1', 'b2'] },
-        ],
-        '\\.'
-      );
-
       const onSelectStory = jest.fn();
       const wrap = mount(
         <Stories
-          storiesHierarchy={data}
+          storiesHierarchy={dataWithSeparator}
           selectedKind="some.name.item1"
           selectedStory="a2"
           selectedHierarchy={['some', 'name', 'item1']}