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

Wrapper component instead of an enhancer function (HOC) #16

Closed
zaygraveyard opened this issue Jun 20, 2016 · 3 comments
Closed

Wrapper component instead of an enhancer function (HOC) #16

zaygraveyard opened this issue Jun 20, 2016 · 3 comments

Comments

@zaygraveyard
Copy link
Contributor

First-off thank you for this great library.
I recently realized that it better to have a wrapper component (like what React-Draggable does) then to have an HOC.
And so I was wandering what is the reasoning behind this choice, and if it's possible to have a non HOC version of the wrappers.

I would be happy to do a PR to add this feature.

Advantages to using a wrapper component

  • No need for withRef option.
  • Can be used to implement HOC.
  • No displayName generation (=> cleaner in the dev-tools panel)

Usage example

const SortableItem = ({value}) => <li>{value}</li>;
const SortableList = ({items}) => (
    <ul>
        {items.map((value, index) => 
            <SortableElement key={`item-${index}`} index={index}>
                <SortableItem value={value} />
            </SortableElement>
        )}
    </ul>
);

class SortableComponent extends Component {
    state = {
        items: ['Item 1', 'Item 2', 'Item 3', 'Item 4', 'Item 5', 'Item 6']
    }
    onSortEnd = ({oldIndex, newIndex}) => {
        this.setState({
            items: arrayMove(this.state.items, oldIndex, newIndex)
        });
    };
    render() {
        return (
            <SortableContainer onSortEnd={this.onSortEnd}>
                <SortableList items={this.state.items} />
            </SortableContainer>
        )
    }
}

or

const SortableItem = ({value, ...props}) => (
    <SortableElement {...props}>
        <li>{value}</li>
    </SortableElement>
);
const SortableList = ({items, ...props}) => (
    <SortableContainer {...props}>
        <ul>
            {items.map((value, index) => 
                <SortableItem key={`item-${index}`} index={index} value={value} />
            )}
        </ul>
    </SortableContainer>
);

class SortableComponent extends Component {
    state = {
        items: ['Item 1', 'Item 2', 'Item 3', 'Item 4', 'Item 5', 'Item 6']
    }
    onSortEnd = ({oldIndex, newIndex}) => {
        this.setState({
            items: arrayMove(this.state.items, oldIndex, newIndex)
        });
    };
    render() {
        return (
            <SortableList items={this.state.items} onSortEnd={this.onSortEnd} />
        )
    }
}
@dbachrach
Copy link

I'm finding the component way easier as well. I'm using this:

import React, { PropTypes } from 'react';
import { SortableContainer, SortableElement, SortableHandle } from 'react-sortable-hoc';

export const SortHandle  = SortableHandle(({ children }) => children);
export const SortElement = SortableElement(({ children }) => children);
export const SortContainer = SortableContainer(({ children, wrapper = 'div' }) =>
  React.createElement(wrapper, { children })
);

SortHandle.propTypes = { children: PropTypes.node };

SortElement.propTypes = { children: PropTypes.node };

SortElement.propTypes = {
  children: PropTypes.node,
  wrapper: PropTypes.string.isRequired
};

@clauderic
Copy link
Owner

clauderic commented Aug 31, 2016

Hey @dbachrach, thanks for sharing this. I think it's a neat approach for people who'd rather go down the component route.

Having said that, the idea is to keep react-sortable-hoc as flexible as possible to ensure compatibility with as many other components / libraries as possible, which is why I still privilege the HOC approach.

I'm going to close this issue because I see this as a non-issue and don't want newcomers to be under the impression that there are plans to move in this direction. Feel free to re-open with a PR if you feel strongly about this, and we can discuss there.

@dbachrach
Copy link

@clauderic Sounds fair. I mostly wanted to post this for anyone in the future looking to go down the component route. Thanks.

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

No branches or pull requests

3 participants