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

FIX : New index count is calculating wrong. #207

Closed
wants to merge 2 commits into from
Closed

FIX : New index count is calculating wrong. #207

wants to merge 2 commits into from

Conversation

emrahtoy
Copy link

@emrahtoy emrahtoy commented Jan 5, 2015

When we change the index of an element in sortable list to forward ( ex : 0 to 4 ) it always turns newIndex+1 ( ex : 5 instead of 4 ) but it works good when we move to backward in list. So if we check newIndex>startIndex then decrease the nexIndex -1 it gives right result.

When we change the index of an element in sortable list to forward ( ex : 0 to 4 ) it always turns newIndex+1 ( ex : 5 instead of 4 ) but it works good when we move to backward in list. So if we check newIndex>startIndex then decrease the nexIndex -1 it gives right result.
@dandv
Copy link
Contributor

dandv commented Jan 5, 2015

I've never seen this problem at http://rubaxa-sortable.meteor.com/ (see the console - it displays the indices of the element after reordering)

@emrahtoy
Copy link
Author

emrahtoy commented Jan 5, 2015

Thats interesting after i change the structure of

  • elements and the row collapser of list the problem has been gone and never able to create the same situation.

    Sorry for bothering all of you.

  • @emrahtoy emrahtoy closed this Jan 5, 2015
    @emrahtoy emrahtoy deleted the patch-1 branch January 5, 2015 14:56
    @emrahtoy emrahtoy restored the patch-1 branch January 5, 2015 15:27
    @emrahtoy emrahtoy reopened this Jan 5, 2015
    @emrahtoy
    Copy link
    Author

    emrahtoy commented Jan 5, 2015

    I managed to create situation again. It is only happening pull option set to 'clone';

    @dandv
    Copy link
    Contributor

    dandv commented Jan 5, 2015

    A test case on JSBin would be good. We don't have automatic testing yet, but when we'll implement that, it would help to import all the test cases that people have filed.

    @emrahtoy
    Copy link
    Author

    emrahtoy commented Jan 7, 2015

    Here is the jsBin example : http://jsbin.com/dotiyokuqi/2/edit?html,css,js,console,output

    Move first element 3th line. You should see oldIndex :0 newIndex :2, but you will get oldIndex :0 newIndex :3.

    RubaXa added a commit that referenced this pull request Jan 10, 2015
    @RubaXa
    Copy link
    Collaborator

    RubaXa commented Jan 10, 2015

    Try dev-branch: https://github.com/RubaXa/Sortable/blob/dev/Sortable.js#L628-L632
    (This solution is incorrect)

    @RubaXa RubaXa closed this Jan 10, 2015
    RubaXa added a commit that referenced this pull request Jan 26, 2015
    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.

    3 participants