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

CHANGE to use react-split-view 0.1.63 over the fork #956

Merged
merged 6 commits into from
Apr 28, 2017

Conversation

ndelangen
Copy link
Member

What I did

Replaced the fork of react-split-view for the original.

How to test

Run npm install, cd examples/cra-storybook, npm run storybook
Open browser
drag panel size handles

last step fails, needs work.

@usulpro
Copy link
Member

usulpro commented Apr 24, 2017

@ndelangen I started to work on this.
I will continue to push into this branch

@usulpro usulpro self-assigned this Apr 24, 2017
@ndelangen
Copy link
Member Author

Thanks man!!!

@usulpro
Copy link
Member

usulpro commented Apr 25, 2017

What I did additionally

  • Setup react-split-pane:
    -- remove resizerChildren props (not supported in orig repo) in favor of resizerStyle
    -- remove 'hsplit' and 'vsplit' in favor of universal 'usplit'
    -- put resizer ('usplit') inside panels and make them hiding together
    -- adjust styles to keep original appearance (but there're minor changes)

how to test:
-- Open browser
-- drag panel size handles
-- compare with the previos version of UI

  • Setup panels hiding and restoring:
    -- add size prop to react-split-pane's
    -- store panel sizes in localStorage

how to test:
-- drag panel size handles
-- hide panels and restore (Ctrl - Shift - L, Ctrl - Shift - D) - notice how now it works: it should hide proprely and restore to the same size
-- put addons panel to the right (Ctrl - Shift - J) and change size - notice that each size stores separatelly

  • Add feature: "Left panel on Top":
    -- via split prop in react-split-pane

how to test:
At this moment it's only a privatе feature. To test it you need to change const leftPanelOnTop = false; to true in render manually.
-- should work same as downPanelInRight (hide, restore, store size)
we can use this feature later

@ndelangen
Copy link
Member Author

ndelangen commented Apr 26, 2017

I will test this! => works on my machine!

@ndelangen ndelangen added this to the v3.0.0 milestone Apr 26, 2017
# Conflicts:
#	packages/react-storybook/package.json
#	packages/storybook-ui/package.json
@ndelangen
Copy link
Member Author

I resolved the conflicts, should be good to go.

@usulpro
Copy link
Member

usulpro commented Apr 26, 2017

@ndelangen please don't merge for a while. one more commit is coming

@usulpro
Copy link
Member

usulpro commented Apr 26, 2017

issue: when the left/down panel is hidden the resizer still draggable

what I did: add allowResize prop, fix resizerStyle

how to test: hide a panel, try to resize

@ndelangen It's ready now!

@usulpro
Copy link
Member

usulpro commented Apr 26, 2017

Note: By merging this we close the following issues:
#466 #869 #780 #67

and take a step towards a solution: #813 #868 #124

@ndelangen
Copy link
Member Author

I can now resize the left panel, this is new behaviors I think.

Is this something you wanted to do?

@usulpro
Copy link
Member

usulpro commented Apr 28, 2017

I can now resize the left panel, this is new behaviors I think.

Do you mean leftPanelOnTop mode? Normally it was resizable before, I just fixed some issues. The new behavior isn't available for users at this moment.

I think we can finish this PR as it is for now. How best to switch to this mode is the issue of next step

@ndelangen
Copy link
Member Author

@ndelangen
Copy link
Member Author

But I do agree, this is ready to merge 👍

@ndelangen ndelangen merged commit b45186b into master Apr 28, 2017
@ndelangen ndelangen deleted the splitpane-original branch April 28, 2017 08:03
@Hypnosphi
Copy link
Member

@usulpro Do we still want to support stories panel on top at some point in future?

@usulpro
Copy link
Member

usulpro commented Aug 19, 2017

@Hypnosphi I believe it would be a great option for mobile screens. But at this moment this option not exposed to users.
Do you have any ideas about it?

@Hypnosphi
Copy link
Member

Not really, I just was curious do we still need this dead code

@usulpro
Copy link
Member

usulpro commented Aug 20, 2017

There are some details about possible way to use it in this comment #908 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants