Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Fix panels hiding after resizing #67

Conversation

evgenykochetkov
Copy link

Right now if you resize a panel and then try to hide it(by pressing the shortcut), it will only hide panel's contents, the panel itself still occupies space.

This PR fixes it by not using SplitPane when we need to render just one panel.

Another approach would be to add support for hiding panels in react-split-pane. Not sure which one is the best.

@arunoda
Copy link
Contributor

arunoda commented Jan 11, 2017

This is great.
I think this approach is fine.

I need only small UI fix before we could get this.
see:
screen shot 2017-01-11 at 6 15 06 am

Here we need to add some more margin to the panel in the right.

Copy link
Contributor

@benediktvaldez benediktvaldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to run fine, and fix the issue, so good job on that!

I did try running it on an existing project I have, where I'm using the options addon, and noticed it seems to be causing problems when using either showLeftPanel or showDownPanel, as in the keyboard shortcut won’t work anymore (regardless of the option set, you’re stuck with the option set by the options addon).

I created a repo where you can replicate the problem
https://github.com/benediktvaldez/storybook-ui-67

@@ -20,7 +20,7 @@ const downPanelStyle = {
position: 'absolute',
width: '100%',
height: '100%',
padding: '5px 10px 10px 0',
padding: '0px 10px 10px 0',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering why you changed this style? Doesn't seem to affect the functionality (at first glance) but results in an inconsistency in the layout.

Before: http://d.pr/i/MFLx/4fZwjSJs+

Before After

@usulpro
Copy link
Member

usulpro commented Apr 2, 2017

I tried to reproduce this issue with storybook-addon-options and find out that every changing of showLeftPanel or showDownPanel cause the page reloading

It happens when leftPanel() or downPanel() appears in another place of DOM structure.
The result of page reloading is re-running the code from .storybook\config.js.

That's why

you’re stuck with the option set by the options addon

I believe that the page reloading after keyboard shortcut applying is something undesirable 😮

some minor experiments showed that we can prevent reloading if keep the nesting div structure or something around this... maybe it's possible to find any better solution for this..? 😊

Anyway it seems that it's more issue with this PR than with storybook-addon-options?

BTW react-split-pane is now available as a separated package. I didn't try it deeply but maybe the latest version already solves this issue?

</SplitPane>
{
showLeftPanel
? (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this causes a page reload

ndelangen pushed a commit that referenced this pull request Apr 2, 2017
Added support for require.context similar to storyshots 2.x versions.
@usulpro
Copy link
Member

usulpro commented Apr 10, 2017

Looks like this PR #59 solved the similar issue

@ndelangen
Copy link
Contributor

Hey @evgenykochetkov

Thank you so much for this PR! We're moving over to a mono-repo, since that makes a lot of sense for a project like this.
We would ❤️ it if you could create a PR here: https://github.com/storybooks/storybook

That'd be awesome, thanks!

@ndelangen ndelangen closed this Apr 15, 2017
@evgenykochetkov
Copy link
Author

Oh hi @ndelangen!
I was just about to close this PR anyway because I'd rather try a different approach to this issue.
Great news about the mono-repo, I will gladly do a PR there ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants