Merge pull request #6480 from storybooks/fix/addon-knobs-tab-sorting

Fix sorting of knobs Panels
storybookjs · Apr 14, 2019 · 38b82b8 · 38b82b8
1 parent c9c55f4
commit 38b82b8
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 18 deletions.
diff --git a/addons/knobs/src/components/Panel.js b/addons/knobs/src/components/Panel.js
@@ -14,7 +14,7 @@ import PropForm from './PropForm';
 
 const getTimestamp = () => +new Date();
 
-const DEFAULT_GROUP_ID = 'Other';
+export const DEFAULT_GROUP_ID = 'Other';
 
 const PanelWrapper = styled.div({
   height: '100%',
@@ -186,9 +186,18 @@ export default class KnobPanel extends PureComponent {
       );
     }
 
-    const entries = Object.entries(groups);
-    // Always sort 'Other' (ungrouped) tab last without changing the remaining tabs
-    entries.sort((a, b) => (a[0] === 'Other' ? 1 : 0)); // eslint-disable-line no-unused-vars
+    // Always sort DEFAULT_GROUP_ID (ungrouped) tab last without changing the remaining tabs
+    const sortEntries = g => {
+      const unsortedKeys = Object.keys(g);
+      if (unsortedKeys.indexOf(DEFAULT_GROUP_ID) !== -1) {
+        const sortedKeys = unsortedKeys.filter(key => key !== DEFAULT_GROUP_ID);
+        sortedKeys.push(DEFAULT_GROUP_ID);
+        return sortedKeys.map(key => [key, g[key]]);
+      }
+      return Object.entries(g);
+    };
+
+    const entries = sortEntries(groups);
 
     return (
       <PanelWrapper>

diff --git a/addons/knobs/src/components/__tests__/Panel.js b/addons/knobs/src/components/__tests__/Panel.js
@@ -4,7 +4,7 @@ import { STORY_CHANGED } from '@storybook/core-events';
 import { TabsState } from '@storybook/components';
 
 import { ThemeProvider, themes, convert } from '@storybook/theming';
-import Panel from '../Panel';
+import Panel, { DEFAULT_GROUP_ID } from '../Panel';
 import { CHANGE, SET } from '../../shared';
 import PropForm from '../PropForm';
 
@@ -223,7 +223,7 @@ describe('Panel', () => {
       root.unmount();
     });
 
-    it('should have one tab per groupId and an empty Other tab when all are defined', () => {
+    it('should have one tab per groupId when all are defined', () => {
       const root = mount(
         <ThemeProvider theme={convert(themes.light)}>
           <Panel channel={testChannel} api={testApi} active />
@@ -263,7 +263,7 @@ describe('Panel', () => {
       root.unmount();
     });
 
-    it('the Other tab should have its own additional content when there are knobs both with and without a groupId', () => {
+    it(`the ${DEFAULT_GROUP_ID} tab should have its own additional content when there are knobs both with and without a groupId`, () => {
       const root = mount(
         <ThemeProvider theme={convert(themes.light)}>
           <Panel channel={testChannel} api={testApi} active />
@@ -272,18 +272,18 @@ describe('Panel', () => {
 
       testChannel.on.mock.calls[0][1]({
         knobs: {
-          foo: {
-            name: 'foo',
-            defaultValue: 'test',
-            used: true,
-            groupId: 'foo',
-          },
           bar: {
             name: 'bar',
             defaultValue: 'test2',
             used: true,
             // no groupId
           },
+          foo: {
+            name: 'foo',
+            defaultValue: 'test',
+            used: true,
+            groupId: 'foo',
+          },
         },
       });
 
@@ -293,7 +293,7 @@ describe('Panel', () => {
         .find(TabsState)
         .find('button')
         .map(child => child.prop('children'));
-      expect(titles).toEqual(['foo', 'Other']);
+      expect(titles).toEqual(['foo', DEFAULT_GROUP_ID]);
 
       const knobs = wrapper.find(PropForm).map(propForm => propForm.prop('knobs'));
       // there are props with no groupId so Other should also have its own PropForm

diff --git a/addons/knobs/src/components/__tests__/__snapshots__/Panel.js.snap b/addons/knobs/src/components/__tests__/__snapshots__/Panel.js.snap
@@ -17,7 +17,7 @@ Array [
 ]
 `;
 
-exports[`Panel groups should have one tab per groupId and an empty Other tab when all are defined 1`] = `
+exports[`Panel groups should have one tab per groupId when all are defined 1`] = `
 Array [
   .emotion-2 {
   box-sizing: border-box;

diff --git a/examples/official-storybook/stories/addon-knobs.stories.js b/examples/official-storybook/stories/addon-knobs.stories.js
@@ -124,6 +124,9 @@ storiesOf('Addons|Knobs.withKnobs', module)
     // NOTE: the default value must not change - e.g., do not do date('Label', new Date()) or date('Label')
     const defaultBirthday = new Date('Jan 20 2017 GMT+0');
 
+    // Ungrouped
+    const ungrouped = text('Ungrouped', 'Mumble');
+
     // General
     const name = text('Name', 'Storyteller', GROUP_IDS.GENERAL);
     const age = number('Age', 70, { range: true, min: 0, max: 90, step: 5 }, GROUP_IDS.GENERAL);
@@ -154,9 +157,6 @@ storiesOf('Addons|Knobs.withKnobs', module)
       GROUP_IDS.DISPLAY
     );
 
-    // Ungrouped
-    const ungrouped = text('Ungrouped', 'Mumble');
-
     const style = { backgroundColor, ...otherStyles };
 
     const salutation = nice ? 'Nice to meet you!' : 'Leave me alone!';