UI: Fullscreen toggle does not work when all panels are collapsed

[Tomastomaslol]

Issue: #11782

What I did

• When exiting isFullScreen and navigation is not show set showNav to true
• When togglePanel is invoked and the panel should be hidden and navigation is hidden. Set isFullScreen to true
• When toggleNav is invoked and the navigation should be hidden and the panel is hidden. Set isFullScreen to true

How to test

The expected behaviour is described here: #11782 (comment)

Scenario 1

• Hide Panel and Nav
• Should Enter fullscreen mode automatically
• Exit fullscreen mode
• Navigation should be shown

Scenario 2

• Hide Nav
• Enter fullscreen mode
• Exit fullscreen mode
• Navigation and panel should be shown

Scenario 3

• Hide Panel
• Enter fullscreen mode
• Exit fullscreen mode
• Navigation should be shown

Scenario 4

• Enter fullscreen mode
• Exit fullscreen mode
• Navigation and panel should be shown

[yannbf]

Awesome work @Tomastomaslol !!!

I'm just wondering about scenario 3:

Hide Panel
Enter fullscreen mode
Exit fullscreen mode
Navigation should be shown

If we check @domyen's explanation:

When you press the close button in full screen mode, it should open the sidebar. If the addon panel was previous open it should take you back to it's previous state. If the previous state of the sidebar or addon panel can't be determined, reset the manager UI.

So in this case, at least from what I read, when exiting full screen mode, having the panel being hidden previously, it should go back to the previous state, which is panel open, navigation hidden. Please correct me if I'm wrong @domyen.

Also, there was also a nice suggestion to give a hint of the shortcuts in the buttons:

"Go full screen [F]"

"Change addon orientation [D]
"Hide addons [A]"

Would you be so kind to add that as well?

[Tomastomaslol]

Hi @yannbf. I appreciate you taking the time to look at my pull request. I updated the titles I missed in my previous commit.

Here is how I interpreter the instructions:

When you press the close button in full screen mode, it should open the sidebar.

So exiting full-screen mode should always open the sidebar.

If the addon panel was previous open it should take you back to it's previous state.

Does this mean the state previous to when the full-screen mode was toggled? If so I believe the my suggested implementation is correct.

But if this means its own previous state if the state has changed, I need to give this another go.

Please let me know if I misunderstood anything or if you would like to change the suggested implementation.

[ndelangen]

these are not hard coded, but rather user configurable!

[yannbf]

I assume to get them it's kinda like this?

import { Consumer } from '@storybook/api';

const Component = () => (
  <Consumer>
    {({ api }) => {
      const shortcuts = api.getShortcutKeys();
      return (
        <>
          <ComponentForFullScreen title={`Toggle fullscreen [${shortcuts.fullScreen}]`} />;
          <ComponentForAddonOrientation
            title={`Change addon orientation [${shortcuts.panelPosition}]`}
          />
          ;
          <ComponentForHidingAddons title={`Hide addons [${shortcuts.togglePanel}]`} />;
        </>
      );
    }}
  </Consumer>
);

Or is there another way to get them?

[Tomastomaslol]

Thanks for the feedback! I fixed this issue in my latest commit please let me what you think of my proposed solution.

[ndelangen]

hotkeys are configurable

[yannbf]

It all seems good to me!
@domyen could you please double check this PR, especially this comment? Thanks!

[shilman]

@Tomastomaslol it looks like CI is failing. can you take a look?

Error: Evaluation failed: TypeError: Cannot read property 'getShortcutKeys' of undefined
    at Object.PanelMapper [as current] (https://5a375b97f4b14f0020b0cda3-chgwvaljgf.chromatic.com/main.2dee079d08dfc61f7a62.bundle.js:1:462099)
    at ManagerConsumer (webpack://storybook_docs_dll//tmp/storybook/lib/api/dist/index.js?:624:23)

[domyen]

Will have a chance to take a look today. Thanks for the heads up Yann.

…

On Tue, Sep 8, 2020 at 8:49 AM Michael Shilman ***@***.***> wrote: @Tomastomaslol <https://github.com/Tomastomaslol> it looks like CI is failing. can you take a look? Error: Evaluation failed: TypeError: Cannot read property 'getShortcutKeys' of undefined at Object.PanelMapper [as current] (https://5a375b97f4b14f0020b0cda3-chgwvaljgf.chromatic.com/main.2dee079d08dfc61f7a62.bundle.js:1:462099) at ManagerConsumer (webpack://storybook_docs_dll//tmp/storybook/lib/api/dist/index.js?:624:23) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11810 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACAJWLLPKCMYOAPGHUSEKLSEYR47ANCNFSM4PVEB5OQ> .

[domyen]

@Tomastomaslol thanks so much for this PR! 🙇‍♂️

The gifs are super helpful.

If the addon panel was previous open it should take you back to it's previous state.

Does this mean the state previous to when the full-screen mode was toggled? If so I believe the my suggested implementation is correct.

✅ I think you got it. The deploy previews are down so it's tough to verify the behavior.

✅Example A:

1. Rest state is the sidebar and addon panel are shown
2. User goes full screen, sidebar and addon panel are hidden
3. User exits full screen, sidebar and addon panel are shown

✅Example B:

1. Rest state is the sidebar is shown and addon panel is hidden
2. User goes full screen, sidebar is hidden and addon panel continues to be hidden
3. User exits full screen, sidebar is shown and addon panel continues to be hidden

Also, thank you @yannbf for guiding this issue and PR home 🙏

[Tomastomaslol]

@shilman Thanks for pointing out the failing tests. In the future, I will pay more attention to checks and its outcomes to make sure it hasn't flagged any issues in my PR.

While fixing the tests I realised there were better ways to structure my code so I did some refactoring. There should be no change to functionality.

@domyen I appreciate you taking time look at my code and helping me verify the behaviour!

And thanks for all the help @yannbf. I had a lot of fun working on this! 🙂

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[domyen]

This is ready to go from my end pending CI passing 👍