Merge pull request #5106 from Kovensky/fix-4829

Fix #4829: render non-grouped knobs in the ALL tab

addons/knobs/src/components/Panel.js

@@ -23,7 +23,9 @@ const PanelWrapper = styled.div({
 export default class Panel extends PureComponent {
   constructor(props) {
     super(props);
-    this.state = { knobs: {} };
+    this.state = {
+      knobs: {},
+    };
     this.options = {};
 
     this.lastEdit = getTimestamp();
@@ -136,9 +138,9 @@ export default class Panel extends PureComponent {
     const groups = {};
     const groupIds = [];
 
-    let knobsArray = Object.keys(knobs).filter(key => knobs[key].used);
+    const knobKeysArray = Object.keys(knobs).filter(key => knobs[key].used);
 
-    knobsArray
+    knobKeysArray
       .filter(key => knobs[key].groupId)
       .forEach(key => {
         const knobKeyGroupId = knobs[key].groupId;
@@ -147,6 +149,8 @@ export default class Panel extends PureComponent {
           render: ({ active: groupActive, selected }) => (
             <TabWrapper active={groupActive || selected === DEFAULT_GROUP_ID}>
               <PropForm
+                // false positive
+                // eslint-disable-next-line no-use-before-define
                 knobs={knobsArray.filter(knob => knob.groupId === knobKeyGroupId)}
                 onFieldChange={this.handleChange}
                 onFieldClick={this.handleClick}
@@ -158,11 +162,30 @@ export default class Panel extends PureComponent {
       });
 
     groups[DEFAULT_GROUP_ID] = {
-      render: () => null,
+      render: ({ active: groupActive }) => {
+        // false positive
+        // eslint-disable-next-line no-use-before-define
+        const defaultKnobs = knobsArray.filter(
+          knob => !knob.groupId || knob.groupId === DEFAULT_GROUP_ID
+        );
+
+        if (defaultKnobs.length === 0) {
+          return null;
+        }
+        return (
+          <TabWrapper active={groupActive}>
+            <PropForm
+              knobs={defaultKnobs}
+              onFieldChange={this.handleChange}
+              onFieldClick={this.handleClick}
+            />
+          </TabWrapper>
+        );
+      },
       title: DEFAULT_GROUP_ID,
     };
 
-    knobsArray = knobsArray.map(key => knobs[key]);
+    const knobsArray = knobKeysArray.map(key => knobs[key]);
 
     if (knobsArray.length === 0) {
       return <Placeholder>NO KNOBS</Placeholder>;

addons/knobs/src/components/__tests__/Panel.js

@@ -1,6 +1,8 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
+import { TabsState } from '@storybook/components';
 import Panel from '../Panel';
+import PropForm from '../PropForm';
 
 describe('Panel', () => {
   it('should subscribe to setKnobs event of channel', () => {
@@ -144,4 +146,119 @@ describe('Panel', () => {
       // expect(testApi.setQueryParams).toHaveBeenCalledWith(paramsChange);
     });
   });
+
+  describe('groups', () => {
+    const testChannel = {
+      on: jest.fn(),
+      emit: jest.fn(),
+      removeListener: jest.fn(),
+    };
+    const testApi = {
+      getQueryParam: jest.fn(),
+      setQueryParams: jest.fn(),
+      onStory: jest.fn(() => () => {}),
+    };
+
+    it('should have no tabs when there are no groupIds', () => {
+      // Unfortunately, a shallow render will not invoke the render() function of the groups --
+      // it thinks they are unnamed function components (what they effectively are anyway).
+      //
+      // We have to do a full mount.
+
+      const wrapper = mount(<Panel channel={testChannel} api={testApi} active />);
+      try {
+        wrapper.setState({
+          knobs: {
+            foo: {
+              name: 'foo',
+              defaultValue: 'test',
+              used: true,
+              // no groupId
+            },
+          },
+        });
+
+        expect(wrapper.find(TabsState).exists()).toBeFalsy();
+
+        const formWrapper = wrapper.find(PropForm);
+        const knobs = formWrapper.map(formInstanceWrapper => formInstanceWrapper.prop('knobs'));
+        expect(knobs).toMatchSnapshot();
+      } finally {
+        wrapper.unmount();
+      }
+    });
+
+    it('should have one tab per groupId and an empty ALL tab when all are defined', () => {
+      const wrapper = mount(<Panel channel={testChannel} api={testApi} active />);
+      try {
+        wrapper.setState({
+          knobs: {
+            foo: {
+              name: 'foo',
+              defaultValue: 'test',
+              used: true,
+              groupId: 'foo',
+            },
+            bar: {
+              name: 'bar',
+              defaultValue: 'test2',
+              used: true,
+              groupId: 'bar',
+            },
+          },
+        });
+
+        const titles = wrapper
+          .find(TabsState)
+          // TabsState will replace the <div/> that Panel actually makes with a <Tab/>
+          .find('Tab')
+          .map(child => child.prop('name'));
+        // the "ALL" tab is always defined
+        expect(titles).toEqual(['foo', 'bar', 'ALL']);
+
+        const knobs = wrapper.find(PropForm).map(propForm => propForm.prop('knobs'));
+        // but it should not have its own PropForm in this case
+        expect(knobs).toHaveLength(titles.length - 1);
+        expect(knobs).toMatchSnapshot();
+      } finally {
+        wrapper.unmount();
+      }
+    });
+
+    it('the ALL tab should have its own additional content when there are knobs both with and without a groupId', () => {
+      const wrapper = mount(<Panel channel={testChannel} api={testApi} active />);
+      try {
+        wrapper.setState({
+          knobs: {
+            foo: {
+              name: 'foo',
+              defaultValue: 'test',
+              used: true,
+              groupId: 'foo',
+            },
+            bar: {
+              name: 'bar',
+              defaultValue: 'test2',
+              used: true,
+              // no groupId
+            },
+          },
+        });
+
+        const titles = wrapper
+          .find(TabsState)
+          // TabsState will replace the <div/> that Panel actually makes with a <Tab/>
+          .find('Tab')
+          .map(child => child.prop('name'));
+        expect(titles).toEqual(['foo', 'ALL']);
+
+        const knobs = wrapper.find(PropForm).map(propForm => propForm.prop('knobs'));
+        // there are props with no groupId so ALL should also have its own PropForm
+        expect(knobs).toHaveLength(titles.length);
+        expect(knobs).toMatchSnapshot();
+      } finally {
+        wrapper.unmount();
+      }
+    });
+  });
 });

addons/knobs/src/components/__tests__/__snapshots__/Panel.js.snap

@@ -0,0 +1,54 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Panel groups should have no tabs when there are no groupIds 1`] = `
+Array [
+  Array [
+    Object {
+      "defaultValue": "test",
+      "name": "foo",
+      "used": true,
+    },
+  ],
+]
+`;
+
+exports[`Panel groups should have one tab per groupId and an empty ALL tab when all are defined 1`] = `
+Array [
+  Array [
+    Object {
+      "defaultValue": "test",
+      "groupId": "foo",
+      "name": "foo",
+      "used": true,
+    },
+  ],
+  Array [
+    Object {
+      "defaultValue": "test2",
+      "groupId": "bar",
+      "name": "bar",
+      "used": true,
+    },
+  ],
+]
+`;
+
+exports[`Panel groups the ALL tab should have its own additional content when there are knobs both with and without a groupId 1`] = `
+Array [
+  Array [
+    Object {
+      "defaultValue": "test",
+      "groupId": "foo",
+      "name": "foo",
+      "used": true,
+    },
+  ],
+  Array [
+    Object {
+      "defaultValue": "test2",
+      "name": "bar",
+      "used": true,
+    },
+  ],
+]
+`;