Skip to content
This repository has been archived by the owner on Sep 8, 2020. It is now read-only.

Allow ui-sortable to work on deep items #524

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chrysn
Copy link

@chrysn chrysn commented Jun 20, 2017

The sortable container might not be the item's direct parent; this
happens eg. in floating setups when the ng-repeated DOM node is a
directive that only includes the floating node as a child, so the
resulting tree with directives and CSS resolved looks like

<div ng-model="items" ui-sortable="{items:'.tilefloater'}">
  <tile ng-repeat="i in items>
    <tile-implementation class="tilefloater" sytle="float:left" ... />
  </tile>
</div>

I'm unsure on why the .find(opts['ui-model-items']) ->
.find(opts['items']) is necessary exactly (or why it doesn't say the
latter originally), but it is required in the same situation; were I to
(just on a guess) set {ui-model-items:'.tilefloater',
items:'.tilefloater'}, there'd be leftover placeholders, and without the
change, .find() doesn't catch the actually moved item.

The sortable container might not be the item's direct parent; this
happens eg. in floating setups when the ng-repeated DOM node is a
directive that only includes the floating node as a child, so the
resulting tree with directives and CSS resolved looks like

    <div ng-model="items" ui-sortable="{items:'.tilefloater'}">
      <tile ng-repeat="i in items>
        <tile-implementation class="tilefloater" sytle="float:left" ... />
      </tile>
    </div>

I'm unsure on why the .find(opts['ui-model-items']) ->
.find(opts['items']) is necessary exactly (or why it doesn't say the
latter originally), but it is required in the same situation; were I to
(just on a guess) set {ui-model-items:'.tilefloater',
items:'.tilefloater'}, there'd be leftover placeholders, and without the
change, .find() doesn't catch the actually moved item.
@thgreasi
Copy link
Contributor

Thanks for your PR!
How about using the handle option, that does more or less the same?
Also, in case that you work with transcluded directives, take a look at the Integrating with directives doing transclusion section of README.

@thgreasi
Copy link
Contributor

Any feedback on this?

@chrysn
Copy link
Author

chrysn commented Jun 24, 2017 via email

@chrysn
Copy link
Author

chrysn commented Jun 26, 2017

ad handle: I do use handle already, but that seems unrelated to me.
ad example: I've stripped down the example to https://codepen.io/anon/pen/zzEbbe to demonstrate the issue. the ng-repeat is on an outer div (that's not styled by itself), and only an inner div is styled as float; it has a distinct handle.

@thgreasi
Copy link
Contributor

Can you add the a set of curly in your while statement to fix the linter errors and see whether this breaks any tests?
Can you share some details about why the sorted items can't be under the same parent?
For now, the official way to achieve such effects would be by using the helper and placeholder options.

@chrysn
Copy link
Author

chrysn commented Jul 3, 2017

Updated with braces; commits not smushed for better review. I don't know how to interpret the current test failure.

ad "why not same parent": The items are not under the same parent because the angular directives that provide the (conditionally floating) items leave their own tags inbetween.

@thgreasi
Copy link
Contributor

thgreasi commented Jul 3, 2017

I'm afraid that this might break things added on v0.17 that added support for transcluded directives.
That's a really strange error! Does it also throw the same error locally?

@chrysn
Copy link
Author

chrysn commented Jul 6, 2017

I get the same error after I pulled in the whole test suite.

Is there any way to run the test suite in a more direct way than encapsulated in karma, to drill down on what makes things go wrong?

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

Successfully merging this pull request may close these issues.

2 participants