Skip to content

Adding index sortable example #2284

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

Closed

Conversation

aschmoe
Copy link

@aschmoe aschmoe commented Jan 12, 2018

RE: #91

Adds react-sortable-hoc example, adds index prop to ValueComponent

Work largely done by @pwhipp

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.973% when pulling 35e895a on aschmoe:adding-index-sortable-example into 851d94b on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.973% when pulling 35e895a on aschmoe:adding-index-sortable-example into 851d94b on JedWatson:master.

@JedWatson
Copy link
Owner

@aschmoe this looks great! I'm about to publish a new patch version with other small fixes, and don't have time to review this fully before then but will come back to it asap.

Would you mind amending the PR to not include the changes to the build files please? dist and lib get updated when I publish new versions, including them in PRs can be problematic. We just need the changes to the src and style folders.

Thanks!

@aschmoe aschmoe force-pushed the adding-index-sortable-example branch from 35e895a to 8c308fa Compare January 15, 2018 18:58
@aschmoe
Copy link
Author

aschmoe commented Jan 15, 2018

@JedWatson build files have been removed (though I've just realized not examples/dist so I'm removing that as well until I hear otherwise)

@aschmoe aschmoe force-pushed the adding-index-sortable-example branch from 8c308fa to acacbdd Compare January 15, 2018 19:02
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.973% when pulling 6c2dc9c on aschmoe:adding-index-sortable-example into 82910d9 on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.973% when pulling 6c2dc9c on aschmoe:adding-index-sortable-example into 82910d9 on JedWatson:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.973% when pulling 6c2dc9c on aschmoe:adding-index-sortable-example into 82910d9 on JedWatson:master.

@wimoMisterX
Copy link

@JedWatson when could I expect a release with this PR being merged?

@liegeandlief
Copy link

liegeandlief commented Feb 7, 2018

@aschmoe - Really like this! One thing I noticed is that when clicking on an option to drag it, it causes the select input to open. Is it possible/desirable to prevent this, perhaps using the onSortStart option in react-sortable-hoc?

Copy link
Collaborator

@jossmac jossmac left a comment

Choose a reason for hiding this comment

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

LGTM @aschmoe

Please resolve the conflict, then this is good to merge

@jossmac jossmac added v1 labels Jul 4, 2018
@elyobo
Copy link

elyobo commented Jul 5, 2018

A useful example to have, can we get this conflict resolved so that it can be merged?

@JedWatson JedWatson changed the base branch from master to sortable-example October 2, 2019 06:19
@JedWatson JedWatson changed the base branch from sortable-example to master October 2, 2019 06:20
@JedWatson
Copy link
Owner

Thanks everyone who had a hand in this - with #3792 it's finally live

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

Successfully merging this pull request may close these issues.

7 participants