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

Incorrect ui.oldindex and ui.item.index() when using items options #48

Closed
lacek opened this issue Oct 24, 2014 · 4 comments · Fixed by #139
Closed

Incorrect ui.oldindex and ui.item.index() when using items options #48

lacek opened this issue Oct 24, 2014 · 4 comments · Fixed by #139
Assignees
Labels
feature New feature or enhancement

Comments

@lacek
Copy link

lacek commented Oct 24, 2014

I am using Ember to {{each}} helper to create a list, which looks like below:

  <ul class="sortable">
    <script></script>
    <script></script>
    <script></script>
    <li>item 1</li>
    <li>item 2</li>
    <li>item 3</li>
    <script></script>
    <script></script>
    <script></script>
  </ul>

All the script tags are Ember-generated as metaphor.

The following script is used to show the change of indexes:

  $('.sortable').sortable({
    items: 'li'
  }).bind('sortupdate', function(e, data) {
    $('#change').text(data.oldindex + ' -> ' + data.item.index());
  });

Both data.oldindex and data.item.index() are shift by 3 due to the script tags. Is this behavior intended or unexpected?

JSBin: http://jsbin.com/sidugi/edit

@lacek
Copy link
Author

lacek commented Oct 28, 2014

For Ember users who may have the same problem, my workaround is to use CollectionView to generate the ul: http://jsbin.com/lemahu/4/edit

@lukasoppermann
Copy link
Owner

Wouldn't it be sensible to do it this way, because if you want to add an item to the list the actual position it should be appended to is the index you get.

I don't know ember, and the <script> tags seem a little weird to me, but if you have a list like the following I would think an index should include the disabled item, so when you add a ne item with js (like moving to a connected list) it gets the correct position.

<ul>
  <li class="disabled">...<li>
  <li>...</li>
  <li>...</li>
  <li>...</li>
</ul>

@lacek
Copy link
Author

lacek commented Dec 26, 2014

@lukasoppermann I agree that it's weird ember generates such script tags and they're actually breaking the HTML structure. Nevertheless, since the items option is for specifying which items inside the element should be sortable, I naturally think that the index is referring to position among the sortable items but in fact it is the position among the item's sibling elements. If this is the original design, I think it should be stated in the documentation.

@lukasoppermann
Copy link
Owner

@lacek I see what you mean, as I thought the same for a moment, but consider you want to add an item into your list at the visual index 2. If you would use index: 2 it would be added before the visible items, so it actually makes sense to calculate the "real index".

Do you have a use-case where it is necessary to have the index of only the items? I guess you could always send a PR adding another value to the event like items-index or maybe something with a better name.

I do also agree on a need to document this behaviour so maybe you want to send a separate PR for this?

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

Successfully merging a pull request may close this issue.

2 participants