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

Split Pane v2 - simplified API #240

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

Split Pane v2 - simplified API #240

wants to merge 53 commits into from

Conversation

tomkp
Copy link
Owner

@tomkp tomkp commented Dec 31, 2017

This PR is for a new v2 of react-split-pane.

Checkout the demo

Try it out today:

npm install react-split-pane@next

or

yarn add react-split-pane@next

The intention is to simplify the API by allowing Panes within the SplitPane to take props such as minSize, maxSize, initialSize, className, style etc.

<SplitPane split="vertical">
          <Pane>You can use a Pane component</Pane>
          <div>or you can use a plain old div</div>
          <Pane initialSize="25%" minSize="10%" maxSize="500px">Using a Pane allows you to specify any constraints directly</Pane>
</SplitPane>

I'm happy for any / all contributions!

Some things todo (in no particular order)

  • Allow <Pane> or plain old div
  • Multiple panes
  • Minimum size
  • Maximum size
  • Initial size
  • Allow % or px
  • Proportional resizing
  • Initial setup of Storybook
  • Add examples to Storybook
  • Overriding classes
  • Overriding styles
  • TypeScript bindings
  • Step - resize panes by specific increments
  • Snap to position
  • Event handling
  • Test coverage
  • Performance enhancements
  • Allow custom <Resizer>
  • Standardise styling approach (Glamorous vs Styled-Components vs inline-style-prefixer)
  • Update readme
  • Cross browser testing

@tomkp tomkp added the v2 label Dec 31, 2017
@tomkp tomkp mentioned this pull request Dec 31, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-28.3%) to 62.637% when pulling 7fec1bf on v2 into 88fa530 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-29.8%) to 61.194% when pulling 82efd67 on v2 into 88fa530 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.4%) to 86.567% when pulling 58d12cd on v2 into 88fa530 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.4%) to 86.567% when pulling 1dc52b6 on v2 into 88fa530 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 90.672% when pulling f355497 on v2 into 88fa530 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 90.672% when pulling 4107ad4 on v2 into 88fa530 on master.

tomkp and others added 6 commits May 17, 2018 06:55
* use src files in storybook, not lib

* don't break when a child is null

* Remove changes to storybook

* Remove changes to storybook

* Remove changes to storybook
@ndelangen
Copy link

👏

@adamchenwei
Copy link

Go @tomkp go! this is very exciting!

@ndelangen
Copy link

@tomkp could you give me a heads-up when you think this is stable enough to usage in storybook? Happy to test this for you.

With usage you'll likely get more/better feedback.

@ndelangen
Copy link

ndelangen commented Sep 10, 2018

You'll probably want to:

I recommend removing this from your demos:
screen shot 2018-09-10 at 22 49 21

@wuweiweiwu
Copy link
Collaborator

@ndelangen I'll put that on my TODO list. I haven't worked much on v2 but I'll touch base with @tomkp . 👍

@ndelangen
Copy link

I hit some bugs with the minSize, and I think this code right here needs rethinking / optimisation:
https://github.com/tomkp/react-split-pane/blob/v2/src/SplitPane.js#L105

Currently when the parent re-renders, the pane-sizes reset to default.

@walerun
Copy link

walerun commented Sep 11, 2018

Hi @ndelangen, could you provide an example with issue you described?

@tsaiDavid
Copy link

@wuweiweiwu @tomkp - is there something I could help with?

@tomkp
Copy link
Owner Author

tomkp commented Dec 19, 2018

@tsaiDavid Yes! I'd love some help :-)

I'll have a think about what would be most useful - but off the top of my head I think that I was keen to get the ability to listen on events working.

eg:

onDragStarted
This callback is invoked when a drag starts.

onDragFinished
This callback is invoked when a drag ends.

onChange
This callback is invoked with the current drag during a drag event. It is recommended that it is wrapped in a debounce function.

@wuweiweiwu wuweiweiwu added this to the 2.0.0 milestone Jan 18, 2019
@josgraha
Copy link

@wuweiweiwu @tomkp any thoughts of moving the website to Create React App (CRA)?

@twolf2919
Copy link

Hi Tom - awesome demo examples!
But I'm getting exceptions using the splitter. Here's my very simple App.js's render():
render() { return( <SplitPane split="horizontal"> <Pane initialSize="50px" minSize="50px" maxSize="50px"> Navigation </Pane> <Pane> Content </Pane> </SplitPane> ); }
But when I run the app, the browser complains:
`
TypeError: undefined is not an object (evaluating 'child.props['size']')
(anonymous function)
node_modules/react-split-pane/lib/SplitPane.js:329
326 | key: 'getPanePropSize',
327 | value: function getPanePropSize(props) {
328 | return _react2.default.Children.map(props.children, function (child) {

329 | var value = child.props['size'] || child.props['initialSize'];
| ^ 330 |
331 | if (value === undefined) {
332 | return DEFAULT_PANE_SIZE;
`
Any help is much appreciated. I'm pretty much a Javascript/react newbie.

@wuweiweiwu wuweiweiwu self-assigned this Jun 26, 2019
@fsjobeck
Copy link

Hi, just like to say this is probably the smoothes split-pane component i've found out there so far, great job!
Is the v2 being actively work on these days? Seems it's gone a bit stale?

@wuweiweiwu
Copy link
Collaborator

@fsjobeck will pick back up when I get some free time!

@miso-belica
Copy link

miso-belica commented Aug 19, 2020

Ou, this seems abandoned from the previous year what is sad, because the API looks nice and also it seems working pretty good. I wanted to mention only 1 issue I had: The example import in the https://www.npmjs.com/package/react-split-pane#new-version does not work. I needed to import Pane like this import Pane from 'react-split-pane/lib/Pane'.

Hope this will be finished someday. It's a nice library and also this PR. Thanks.

index: idx,
'data-type': 'Pane',
split: split,
key: `Pane-${idx}`,
Copy link

@alexisthual alexisthual Dec 9, 2020

Choose a reason for hiding this comment

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

To me, an interesting improvement would be to let the user set the key for Pane components.
I implemented an add / remove logic for panes using a DOM structure similar to

<SplitPane>
  {panes.map((paneId: string) => 
    <Pane key={`scene-pane-${paneId}`}>
        ...
    </Pane>
  }
</SplitPane>

In my example, panes is a state variable consisting of a list of unique pane identifiers. When I close a pane, a callback simply removes its id from the list. However, because this librairy rewrites key for Pane, I get inconsistent results.
Please consider using something like nanoid to generate unique ids for panes rather than using their position in the list of panes.

@shakiba
Copy link

shakiba commented Feb 28, 2021

It would be great if this can be used with CSSTransition.

@vishal-vc
Copy link

@tomkp any update on this release

@daveleee
Copy link

Ou, this seems abandoned from the previous year what is sad, because the API looks nice and also it seems working pretty good. I wanted to mention only 1 issue I had: The example import in the https://www.npmjs.com/package/react-split-pane#new-version does not work. I needed to import Pane like this import Pane from 'react-split-pane/lib/Pane'.

Hope this will be finished someday. It's a nice library and also this PR. Thanks.

@miso-belica
Thanks man, it's such a nice discovery that saved my time 😉

@hqwlkj
Copy link

hqwlkj commented Sep 2, 2023

Could not find a declaration file for module  react-split-pane/lib/Pane 

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

Successfully merging this pull request may close these issues.