From 7edfeb0ee292dd6535cce2cca56e44fe4ee8cad6 Mon Sep 17 00:00:00 2001
From: Benedikt D Valdez <benediktvaldez@gmail.com>
Date: Thu, 11 Apr 2019 14:02:57 +0000
Subject: [PATCH] Refactor sorting implementation to fix tests

---
 addons/knobs/src/components/Panel.js          | 17 +++++++++++-----
 .../knobs/src/components/__tests__/Panel.js   | 20 +++++++++----------
 .../__tests__/__snapshots__/Panel.js.snap     |  2 +-
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/addons/knobs/src/components/Panel.js b/addons/knobs/src/components/Panel.js
index 0773bc9be01d..f2a090e82607 100644
--- a/addons/knobs/src/components/Panel.js
+++ b/addons/knobs/src/components/Panel.js
@@ -21,7 +21,7 @@ import PropForm from './PropForm';
 
 const getTimestamp = () => +new Date();
 
-const DEFAULT_GROUP_ID = 'Other';
+export const DEFAULT_GROUP_ID = 'Other';
 
 const PanelWrapper = styled(({ children, className }) => (
   <ScrollArea horizontal vertical className={className}>
@@ -198,10 +198,17 @@ export default class KnobPanel extends PureComponent {
     }
 
     // Always sort DEFAULT_GROUP_ID (ungrouped) tab last without changing the remaining tabs
-    const unsortedEntries = Object.entries(groups);
-    const entries = unsortedEntries
-      .filter(entry => entry[0] !== DEFAULT_GROUP_ID)
-      .push(unsortedEntries.find(entry => entry[0] === DEFAULT_GROUP_ID));
+    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 (
       <Fragment>
diff --git a/addons/knobs/src/components/__tests__/Panel.js b/addons/knobs/src/components/__tests__/Panel.js
index 7c44bf58f615..a01dbb2c6595 100644
--- 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';
 
@@ -153,7 +153,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 api={testApi} active />
@@ -193,7 +193,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 api={testApi} active />
@@ -202,18 +202,18 @@ describe('Panel', () => {
 
       testApi.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',
+          },
         },
       });
 
@@ -223,7 +223,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
index 839fe427caa5..df5db4b0130b 100644
--- 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;