Merge pull request mui#5514 from oliviertassinari/list-item-control

[ListItem] Fix an issue with the controlled behavior

Author: oliviertassinari

src/List/ListItem.js

@@ -434,9 +434,21 @@ class ListItem extends Component {
 
   handleNestedListToggle = (event) => {
     event.stopPropagation();
-    this.setState({open: !this.state.open}, () => {
-      this.props.onNestedListToggle(this);
-    });
+
+    if (this.props.open === null) {
+      this.setState({open: !this.state.open}, () => {
+        this.props.onNestedListToggle(this);
+      });
+    } else {
+      // Exposing `this` in the callback is quite a bad API.
+      // I'm doing a one level deep clone to expose a fake state.open.
+      this.props.onNestedListToggle({
+        ...this,
+        state: {
+          open: !this.state.open,
+        },
+      });
+    }
   };
 
   handleRightIconButtonKeyboardFocus = (event, isKeyboardFocused) => {

src/List/ListItem.spec.js

@@ -3,8 +3,9 @@ import React from 'react';
 import {shallow} from 'enzyme';
 import {assert} from 'chai';
 import ListItem from './ListItem';
-import NestedList from './NestedList';
+import EnhancedButton from '../internal/EnhancedButton';
 import getMuiTheme from '../styles/getMuiTheme';
+import NestedList from './NestedList';
 
 describe('<ListItem />', () => {
   const muiTheme = getMuiTheme();
@@ -14,7 +15,7 @@ describe('<ListItem />', () => {
     const wrapper = shallowWithContext(
       <ListItem />
     );
-    const enhancedButton = wrapper.find('EnhancedButton');
+    const enhancedButton = wrapper.find(EnhancedButton);
     assert.ok(enhancedButton.length);
   });
 
@@ -25,7 +26,7 @@ describe('<ListItem />', () => {
         primaryText={testText}
       />
     );
-    const enhancedButton = wrapper.find('EnhancedButton');
+    const enhancedButton = wrapper.find(EnhancedButton);
 
     assert.strictEqual(enhancedButton.children().text(), testText);
   });
@@ -37,7 +38,7 @@ describe('<ListItem />', () => {
         className={testClass}
       />
     );
-    const enhancedButton = wrapper.find('EnhancedButton');
+    const enhancedButton = wrapper.find(EnhancedButton);
     assert.strictEqual(enhancedButton.prop('className'), testClass);
   });
 
@@ -47,7 +48,7 @@ describe('<ListItem />', () => {
         disabled={true}
       />
     );
-    assert.notOk(wrapper.find('EnhancedButton').length, 'should not have an EnhancedButton');
+    assert.notOk(wrapper.find(EnhancedButton).length, 'should not have an EnhancedButton');
   });
 
   it('should display a disabled list-item with a class if specified.', () => {
@@ -59,7 +60,7 @@ describe('<ListItem />', () => {
       />
     );
 
-    assert.notOk(wrapper.find('EnhancedButton').length, 'should not have an EnhancedButton');
+    assert.notOk(wrapper.find(EnhancedButton).length, 'should not have an EnhancedButton');
     assert.strictEqual(wrapper.find(`.${testClass}`).length, 1, 'should have a div with the test class');
   });
 
@@ -75,19 +76,17 @@ describe('<ListItem />', () => {
     assert.strictEqual(wrapper.find(`.${testClass}`).length, 1, 'should have a div with the test class');
   });
 
-  describe('props: primaryTogglesNestedList', () => {
+  describe('prop: primaryTogglesNestedList', () => {
     it('should toggle nested list when true', () => {
       const wrapper = shallowWithContext(
         <ListItem
-          leftCheckbox={<div />}
-          primaryText="Item text"
           primaryTogglesNestedList={true}
           nestedItems={[
-            <ListItem key={1} primaryText="Nested item text" />,
+            <ListItem key={1} />,
           ]}
         />
       );
-      const primaryTextButton = wrapper.find('EnhancedButton');
+      const primaryTextButton = wrapper.find(EnhancedButton);
 
       assert.strictEqual(wrapper.find(NestedList).props().open, false);
 
@@ -101,20 +100,18 @@ describe('<ListItem />', () => {
     it('should not render primary text button when false', () => {
       const wrapper = shallowWithContext(
         <ListItem
-          leftCheckbox={<div />}
-          primaryText="Item text"
           primaryTogglesNestedList={false}
           nestedItems={[
-            <ListItem key={1} primaryText="Nested item text" />,
+            <ListItem key={1} />,
           ]}
         />
       );
 
-      assert.strictEqual(wrapper.filter('EnhancedButton').length, 0);
+      assert.strictEqual(wrapper.filter(EnhancedButton).length, 0);
     });
   });
 
-  describe('props: open', () => {
+  describe('prop: open', () => {
     it('should initially open nested list', () => {
       const wrapper = shallowWithContext(
         <ListItem
@@ -145,19 +142,48 @@ describe('<ListItem />', () => {
       });
       assert.strictEqual(wrapper.find(NestedList).props().open, true);
     });
+
+    it('should not control the state', () => {
+      const wrapper = shallowWithContext(
+        <ListItem
+          initiallyOpen={false}
+          primaryTogglesNestedList={true}
+          nestedItems={[
+            <ListItem key={1} />,
+          ]}
+        />
+      );
+
+      const primaryTextButton = wrapper.find(EnhancedButton);
+      primaryTextButton.simulate('touchTap', {stopPropagation: () => {}});
+      assert.strictEqual(wrapper.find(NestedList).props().open, true);
+    });
+
+    it('should control the state', () => {
+      const wrapper = shallowWithContext(
+        <ListItem
+          open={false}
+          primaryTogglesNestedList={true}
+          nestedItems={[
+            <ListItem key={1} />,
+          ]}
+        />
+      );
+
+      const primaryTextButton = wrapper.find(EnhancedButton);
+      primaryTextButton.simulate('touchTap', {stopPropagation: () => {}});
+      assert.strictEqual(wrapper.find(NestedList).props().open, false);
+    });
   });
 
-  describe('props: hoverColor', () => {
+  describe('prop: hoverColor', () => {
     it('should use a background color on hover if hoverColor is specified', () => {
       const testColor = '#ededed';
       const wrapper = shallowWithContext(
-        <ListItem
-          hoverColor={testColor}
-        />
+        <ListItem hoverColor={testColor} />
       );
-      wrapper.find('EnhancedButton').simulate('mouseEnter');
-      wrapper.update();
-      assert.strictEqual(wrapper.find('EnhancedButton').prop('style').backgroundColor, testColor);
+      wrapper.find(EnhancedButton).simulate('mouseEnter');
+      assert.strictEqual(wrapper.find(EnhancedButton).props().style.backgroundColor, testColor);
     });
   });
 });