Support more elements inside ui-sortable

[thgreasi]

Change the implementation to match the features of ui-sortable v0.14.x as reported in #10.

[jessefoster]

What help do you need? I added ui-sortable's v0.14.x branch to my project and it worked fine in my limited testing.

[thgreasi]

Actually I'm not sure that we actually need any changes. I have never tested it with the v0.14 branch, so I thought that it wouldn't magically work if we also added some text elements inside the sortable that are not part of the ng-repeat. Do you have a codepen with your use case?

On Mon, Oct 12, 2015, 23:00 Jesse Foster notifications@github.com wrote:

What help do you need? I added ui-sortable's v0.14.x branch to my project and it worked fine in my limited testing.

— Reply to this email directly or view it on GitHub https://github.com/thgreasi/ui-sortable-multiselection/issues/11#issuecomment-147504517 .

Thodoris Greasidis Computer, Networking & Communications Engineer

[thgreasi]

Do you think I should close this and also merge v.14.x-dev into master?

On Tue, Oct 13, 2015, 00:00 Thodoris Greasidis thgreasi@gmail.com wrote:

Actually I'm not sure that we actually need any changes. I have never tested it with the v0.14 branch, so I thought that it wouldn't magically work if we also added some text elements inside the sortable that are not part of the ng-repeat. Do you have a codepen with your use case?

On Mon, Oct 12, 2015, 23:00 Jesse Foster notifications@github.com wrote:

What help do you need? I added ui-sortable's v0.14.x branch to my project and it worked fine in my limited testing.

— Reply to this email directly or view it on GitHub https://github.com/thgreasi/ui-sortable-multiselection/issues/11#issuecomment-147504517 .

Thodoris Greasidis Computer, Networking & Communications Engineer

Thodoris Greasidis Computer, Networking & Communications Engineer

[jessefoster]

I'll set aside some time tomorrow to make a codepen so we have a working example. I didn't look through all your changes in v.14.x-dev so hard for me to make a call on merging. Figure if codepen example works and unit tests are passing then merging is a go.

[jessefoster]

Looks like the branch still has some bugs. I made an example with v.14.x-dev, http://codepen.io/anon/pen/YyxMKm. Moving multiple items from list1 into the dropzone works correctly, however moving multiple items from list2 into list1 breaks. I haven't pinpointed where the bug is but here are steps to reproduce.

• Select items 1a, 2a, 3a
• Drag items by use of 2a
• Items render in second list correctly
• Log models shows lists contain correct items
• Select items 1a, 2a, 3a
• Drag items by use of 2a to top of list 1
• Item 1a and 3a are missing and 2a is correct.
• Log models doesn't worked due to undefined appearing in list 1. Checking scope manually shows, Items 1a and 3a are still in list2.

[thgreasi]

OK, it looks like it's a problem with multiselect... I probably do not use the same method to get the positions of the dragged elements, as in ui-sortable.

I will try to release v0.14 of ui-sortable soon since it seems to be working on is own.

For now it looks like that we can't support more multi-sortable elements except from those that are generated by the signed ng-repeat. I will try to fix it at some point...

Thanks for your interest, your collaboration and the detailed descriptions that you provided!

On Tue, Oct 13, 2015, 21:15 Jesse Foster notifications@github.com wrote:

Looks like the branch still has some bugs. I made an example with v.14.x-dev, http://codepen.io/anon/pen/YyxMKm. Moving multiple items from list1 into the dropzone works correctly, however moving multiple items from list2 into list1 breaks. I haven't pinpointed where the bug is but here are steps to reproduce.

• Select items 1a, 2a, 3a
• Drag items by use of 2a
• Items render in second list correctly
• Log models shows lists contain correct items
• Select items 1a, 2a, 3a
• Drag items by use of 2a to top of list 1
• Item 1a and 3a are missing and 2a is correct.
• Log models doesn't worked due to undefined appearing in list 1. Checking scope manually shows, Items 1a and 3a are still in list2.

— Reply to this email directly or view it on GitHub https://github.com/thgreasi/ui-sortable-multiselection/issues/11#issuecomment-147800402 .

Thodoris Greasidis Computer, Networking & Communications Engineer

[jessefoster]

Ah found the bug, in your helper function you calculate the indices without taking into account 'items' option.

var selectedIndexes = angular.element.map(selectedElements, function (element) {
    return angular.element(element).index();
});

// indexes of the selected siblings
var indexes = angular.element.map(selectedSiblings, function (element) {
    return angular.element(element).index();
});

Two ways to fix this.

1) Get 'items' option from list with ui-sortable directive

var listContainer = item.parent( '[ui-sortable]' );
var itemsOptions = listContainer.sortable( 'option', 'items' )

or

2) Assume ui-sortable-selectable directive is added to any sortable items

var selectedIndexes = angular.element.map(selectedElements, function (element) {
    return listContainer.find( '[ui-sortable-selectable]' ).index( element );
});

While option 2 is required for your directive to work I think option 1 is better as it more tightly integrates with your changes in ui-sortable v14-dev.

Perhaps a long term solution/enhancement is to change directive so it's added to list container like ui-sortable directive. This way you can require 'ui-sortable', add your click events to element specified by 'items' option, and remove the need to add 'ui-sortable-selectable' to all selectable items.

Hope this helps

[thgreasi]

Yea that was more or less what I was thinking. Maybe just copy and use the getItemIndex() from ui-sortable v0.14.x. That should do the job.