Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fullscreen toggle does not work when all panels are collapsed #11782

Closed
yannbf opened this issue Aug 4, 2020 · 14 comments
Closed

Fullscreen toggle does not work when all panels are collapsed #11782

yannbf opened this issue Aug 4, 2020 · 14 comments
Assignees
Labels

Comments

@yannbf
Copy link
Member

yannbf commented Aug 4, 2020

Describe the bug
I am unsure whether this is the intended behavior, but the full screen button does not work when the panels are collapsed.

A bit unrelated to the bug, but it would be great to have a tooltip of some sorts indicating that when the user goes full screen they can press the toggle button or "F", because sometimes people press letters such as "A S D F" by mistake and might feel lost on how to get back to the normal state of which they were before.

To Reproduce
Steps to reproduce the behavior:

  1. Collapse all panels as can be seen on the video below
  2. Click on the full screen toggle button

Expected behavior
Full screen would be toggled back and panels become visible again

Screenshots
storybook issue

Additional context
This has been tested on the official-storybook@next example

@Tomastomaslol
Copy link
Member

Hi @yannbf

Just to clarify.do you expect the bottom panel to reappear when fullscreen mode is toggled and the sidebar is open?

Kapture 2020-08-05 at 8 35 34

Or is this fix only supposed affect the scenario when all panels have been closed and fullscreen mode is toggled?

I would love to try and fix this. Is this issue open for anyone to work on or is it only for a storybook team member?

@shilman
Copy link
Member

shilman commented Aug 5, 2020

@Tomastomaslol we welcome all contributions. any issue that is not assigned is fair game (and if an issue is assigned and you want to grab it, just check with the person it's assigned to -- they might be happy for you to take it!)

@yannbf I guess when everything is hidden and you toggle fullscreen you expect everything to be shown again?

@Tomastomaslol
Copy link
Member

Tomastomaslol commented Aug 5, 2020

Thank you for explaining to me how it works. Could you assign this issue to me? I will try to come up with a solution for it

@Tomastomaslol
Copy link
Member

Could someone please confirm if this is the behaviour we desire:

Scenario 1

  • Hide Panel or Nav
  • Enter fullscreen mode
  • Exit fullscreen mode
  • One panel should be visible

scenarioA

Scenario 2

  • Hide Panel and Nav
  • Enter fullscreen mode
  • Exit fullscreen mode
  • Panel and Nav should be visible

scenarioB

Cheers!

@yannbf
Copy link
Member Author

yannbf commented Aug 7, 2020

Hey @Tomastomaslol great question!
@domyen, what do you think is the best way from the user's perspective? Keep the precedence of closed panels or just toggle them back?

I can see a use case where the user is developing a page on Storybook and wants to use controls in the addons panel but also want to view the page in full screen, so the user could:

  • Hide the navigation
  • Move addons panel to the right to change a few things on controls and save some bottom space
  • Go full screen

When coming back from that (maybe to switch stories or change some controls), which would be the ideal scenario:

1 - User toggles full screen and navigation comes back with addon panel
2 - User toggles full screen and just the addon panel comes back

I created this issue because user could press some keys by mistake and if all panels are closed, full screen will do nothing. But if the intention is that full screen should not affect panels toggled by the user, then it's not a bug but a feature (although I can still see a bit of confusion in that).

@Tomastomaslol
Copy link
Member

I had a think about this and maybe it would be helpful if we tried to redefine the issue. What about if we try to tackle the problem that the UI looks almost identical if all panels are hidden and fullscreen is toggled on and off?

UI state when all panels are hidden, fullscreen mode OFF

Screen Shot 2020-08-08 at 2 25 17 pm

UI state when all panels are hidden, fullscreen mode ON

Screen Shot 2020-08-08 at 2 25 34 pm

Maybe a better solution to try to avoid any confusion would be to disable fullscreen mode when panels are hidden and hide the fullscreen toggle button?

@yannbf
Copy link
Member Author

yannbf commented Aug 10, 2020

Question to think about:

  • from a user standpoint, if you hide all panels, are you then on full screen?

If so, then I think it makes sense to have the full screen button in the "on" state when that happens, so that when the user closes it, it just does its normal behaviour which is to bring back the panels.

@Tomastomaslol
Copy link
Member

Tomastomaslol commented Aug 10, 2020

from a user standpoint, if you hide all panels, are you then on full screen?

That is a great point. Playing around with the UI again I think it might be beneficial to think about the full-screen mode as a separate state from showPanel and showNav.

I can't really come up with anything else that makes sense and would be easy to understand.

At the moment I feel like we have 2 options:

  1. Go with Fullscreen toggle does not work when all panels are collapsed #11782 (comment)
  2. Or do nothing (Which might be the best option, I'm not really sure what I think is the best option)

@yannbf Do you have any alternative suggestions that make sense to you? If not what approach do you think makes the most sense?

@yannbf
Copy link
Member Author

yannbf commented Aug 15, 2020

Hey @Tomastomaslol I guess the main problem here is the scenario of which a user (could be developer or designer/stakeholder checking a component) by mistake, ends up pressing one of the keys: A S D F. It could be that they were trying to type on an input component which was not focused, I don't know. In the case of the panels toggling and the user not understanding what's going on, I personally think there could be some indication for the user.

@domyen what do you think about this? Maybe it could be just a small notification that appears for 3 seconds when the toggle is done for the first time in a user session, giving a quick instruction on how to toggle back. Storybook is a great tool that can be used by non technical people as well, as we shouldn't expect them to know the shortcuts.

Maybe we could even figure out a way to still keep the button with three dots when toggling the side panel, position it somewhere else with low opacity whe not hovered or something like that. It could be the escape hatch for people who don't know what's going on.

Also maybe I'm overthinking and like it was mentioned we could do nothing. What are your perspectives?

@domyen
Copy link
Member

domyen commented Aug 15, 2020

Sorry for the late reply. Was working on the 6.0 launch

from a user standpoint, if you hide all panels, are you then on full screen?

Yes. There's a bug right now. If you hide the addon panel and sidebar the UI doesn't show the full screen "close" button, thus you end up in a dead end not knowing how to get back.

Strawman

  • When the sidebar or addon panels are hidden, it should enter full screen mode.
  • 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.

Anyone interested in making this PR? 🙏

re: Addons in full screen
Not currently a behavior we support. The point of going full screen is to focus solely on your component. Folks can hide the sidebar if they want to see more of the component and the addons panel.

re: notifications
Not sure about adding notifications when full-screen is toggled. I think if we fix the bug it will be more obvious how to exit full screen.

It occurs to me that we can make the keyboard shortcut more obvious by adding it to the title attribute of these buttons. That way when you hover over the full screen button or the addon panel close button it shows it.

image
"Go full screen [F]"

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

@Tomastomaslol
Copy link
Member

@domyen

Anyone interested in making this PR? 🙏

Opened a pull request #11810 and tired to add test examples for all scenarios. Any feedback would be greatly appreciated.

@stale
Copy link

stale bot commented Sep 7, 2020

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!

@stale stale bot added the inactive label Sep 7, 2020
@yannbf yannbf removed the inactive label Sep 15, 2020
@stale
Copy link

stale bot commented Oct 12, 2020

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!

@stale stale bot added the inactive label Oct 12, 2020
@domyen domyen removed the inactive label Oct 13, 2020
@shilman
Copy link
Member

shilman commented Oct 15, 2020

¡Ay Caramba!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.1.0-alpha.24 containing PR #11810 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants