Fix sorting of knobs Panels

[benediktvaldez]

Issue: The panel for ungrouped knobs would not be moved to the end as expected

What I did

• Instead of sorting the array of groups, we now filter the Other panel out of the array completely and append it to the end.
• Updated the story in official-storybook that displays multiple knobs groups so that the ungrouped knob is at the beginning.

The previous implementation of moving the ungrouped knob panel to the end was not working as expected in Chrome. I found an inconsistency in how Chrome handles Array.prototype.sort() compared to the other major browsers. This only happened during development (running start-storybook) and only when the ungrouped knobs were defined before the grouped ones. I think that is the reason this hasn’t been noticed before, since the official-storybook defined the ungrouped knobs last.

Relevant screenshot

How to test

• Is this testable with Jest or Chromatic screenshots?
• Does this need a new example in the kitchen sink apps?
• Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

I'm not sure if any of these apply. I did update the story that should confirm whether this works. To replicate the error:

• run the current version on next locally
• move the ungrouped knob in examples/official-storybook/stories/addon-knobs-stories.js above the other ones (see diff of this PR for reference)
• open UI in Chrome and observe the Other panel on that story move to the front
• (to confirm this fix works): apply the fix in addons/knobs/src/components/Panel.js and observe the problem being fixed. *see diff of this PR for reference)

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fix-addon-knobs-tab-sorting.storybook.now.sh

[shilman]

Thanks for fixing @benediktvaldez! Minor nitpicks above 👍

[shilman]

@benediktvaldez code is looking great but tests are failing. can you take a look?

[ndelangen]

@benediktvaldez

Summary of all failing tests
 FAIL  addons/knobs/src/components/__tests__/Panel.js
  ● Panel › groups › should have one tab per groupId and an empty Other tab when all are defined

    expect(received).toEqual(expected)

    Difference:

    - Expected
    + Received

    - Array [
    -   "foo",
    -   "bar",
    - ]
    + Array []

      254 |         .find('button')
      255 |         .map(child => child.prop('children'));
    > 256 |       expect(titles).toEqual(['foo', 'bar']);
          |                      ^
      257 | 
      258 |       const knobs = wrapper.find(PropForm);
      259 |       // but it should not have its own PropForm in this case

      at Object.toEqual (addons/knobs/src/components/__tests__/Panel.js:256:22)

  ● Panel › groups › the Other tab should have its own additional content when there are knobs both with and without a groupId

    expect(received).toEqual(expected)

    Difference:

    - Expected
    + Received

    - Array [
    -   "foo",
    -   "Other",
    - ]
    + Array []

      294 |         .find('button')
      295 |         .map(child => child.prop('children'));
    > 296 |       expect(titles).toEqual(['foo', 'Other']);
          |                      ^
      297 | 
      298 |       const knobs = wrapper.find(PropForm).map(propForm => propForm.prop('knobs'));
      299 |       // there are props with no groupId so Other should also have its own PropForm

      at Object.toEqual (addons/knobs/src/components/__tests__/Panel.js:296:22)

Let me know if you need any help, great fix btw!

[benediktvaldez]

So I've changed the way I'm fixing this. Part of the problem was that I was doing array.filter(…).push(…) which resulted in a single number which explains the test result you got @ndelangen. But when I fixed that, the tests seemed to completely explode in my face. I believe it has something to do with .filtering and .push-ing the object that seems to break the object in the test environment. I now create an array of the keys, sort them properly and then .map through them to create the entries array.

I also rebased the branch on next again to avoid conflicts.

Great to finally find a way to contribute @ndelangen, I have a few more minor things like this I'm hoping to look into in the next few days if I can find the time.

[shilman]

Looking good! ✨