From 61f6878faf94cd81a73b0681490156f884323651 Mon Sep 17 00:00:00 2001 From: igor Date: Mon, 3 Jul 2017 00:51:03 +0300 Subject: [PATCH 1/3] Fix loss of the state in a reall app with componentWillReceiveProps --- .../left_panel/stories_tree/index.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 842142204c4d..c4bc824aa728 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 @@ -46,6 +46,27 @@ class Stories extends React.Component { }; } + componentWillReceiveProps(nextProps) { + const { selectedHierarchy: nextSelectedHierarchy = [] } = nextProps; + const { selectedHierarchy: currentSelectedHierarchy = [] } = this.props; + + if ( + currentSelectedHierarchy.length === nextSelectedHierarchy.length && + currentSelectedHierarchy.every((item, index) => item === nextSelectedHierarchy[index]) + ) { + return; + } + + const selectedNodes = getSelectedNodes(nextSelectedHierarchy); + + this.setState(prevState => ({ + nodes: { + ...prevState.nodes, + ...selectedNodes, + }, + })); + } + onToggle(node, toggled) { if (node.story) { this.fireOnKindAndStory(node.kind, node.story); From 281b021b0181a220a25d41e3a2549f33b0607d9e Mon Sep 17 00:00:00 2001 From: igor Date: Mon, 3 Jul 2017 09:26:57 +0300 Subject: [PATCH 2/3] Add tests to check the componentWillReceiveProps --- .../left_panel/stories_tree/index.test.js | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) 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 a576674d2342..14aef3420471 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 @@ -151,6 +151,91 @@ describe('manager.ui.components.left_panel.stories', () => { 'some@namespace': true, }); }); + + 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( + + ); + + wrap.setProps({ selectedHierarchy: ['another', 'space', '20'] }); + + const { nodes } = wrap.state(); + + expect(nodes).toEqual({ + 'another@namespace': true, + 'another@space@namespace': true, + 'another@space@20@component': true, + }); + }); + + 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( + + ); + + wrap.setProps({ selectedHierarchy: ['some', 'name', 'item1'] }); + + const { nodes } = wrap.state(); + + expect(nodes).toEqual({ + 'another@namespace': true, + 'another@space@namespace': true, + 'another@space@20@component': true, + 'some@namespace': true, + 'some@name@namespace': true, + 'some@name@item1@component': true, + }); + }); + + 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( + + ); + + const setState = jest.fn(); + wrap.instance().setState = setState; + + wrap.setProps({ selectedHierarchy }); + + expect(setState).not.toHaveBeenCalled(); + }); }); describe('events', () => { From fe3f19df237f0e3290e380d0cb16beedd59749bd Mon Sep 17 00:00:00 2001 From: igor Date: Tue, 4 Jul 2017 12:35:56 +0300 Subject: [PATCH 3/3] Use deepEqual to compare selectedHierarchy --- .../left_panel/stories_tree/index.js | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 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 c4bc824aa728..88b346947434 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 @@ -1,6 +1,7 @@ import { Treebeard } from 'storybook-react-treebeard'; import PropTypes from 'prop-types'; import React from 'react'; +import deepEqual from 'deep-equal'; import treeNodeTypes from './tree_node_type'; import treeDecorators from './tree_decorators'; import treeStyle from './tree_style'; @@ -50,21 +51,16 @@ class Stories extends React.Component { const { selectedHierarchy: nextSelectedHierarchy = [] } = nextProps; const { selectedHierarchy: currentSelectedHierarchy = [] } = this.props; - if ( - currentSelectedHierarchy.length === nextSelectedHierarchy.length && - currentSelectedHierarchy.every((item, index) => item === nextSelectedHierarchy[index]) - ) { - return; - } - - const selectedNodes = getSelectedNodes(nextSelectedHierarchy); + if (!deepEqual(nextSelectedHierarchy, currentSelectedHierarchy)) { + const selectedNodes = getSelectedNodes(nextSelectedHierarchy); - this.setState(prevState => ({ - nodes: { - ...prevState.nodes, - ...selectedNodes, - }, - })); + this.setState(prevState => ({ + nodes: { + ...prevState.nodes, + ...selectedNodes, + }, + })); + } } onToggle(node, toggled) {