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

Fixed to work with deep descendants #494

Closed
wants to merge 2 commits into from
Closed

Conversation

yenoh2
Copy link
Contributor

@yenoh2 yenoh2 commented Nov 29, 2016

In trying to use ui-sortable with Angular Material tabs, I found that it didn't work for two reasons. First you can't have two directives with an isolated scope on the same element. Second, ui-sortable didn't support items that aren't a direct decendant. I modified the "getElementScope" function to get the applicable scope for the draggable item, if the normal code results in null. I also wrapped the md-tabs directive in a div containing the ui-sortable directive to circumvent the two directives with isolated scopes issues.

As far as I can tell, this works. Can you see any issues with my change?

See the following example:
http://codepen.io/yenoh2/pen/rWxxVJ

This seems to be related to "Using ui.sortable with angular ui.bootstrap.tabs have a weird interaction #248"

modifies the sortable.js file at line 198, in order for the ui-sortable directive to work on other directives that have isolated scopes.
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.3%) to 97.653% when pulling 37d04d3 on yenoh2:master into 57c5021 on angular-ui:master.

@thgreasi
Copy link
Contributor

I have already merged this one on a local branch that is going to be v0.16. Have written some v transclusion test cases and trying to make them work.

@thgreasi thgreasi added this to the v0.16.x milestone Dec 1, 2016
@thgreasi thgreasi closed this in #496 Dec 4, 2016
@thgreasi
Copy link
Contributor

thgreasi commented Dec 4, 2016

Merged with #496

@thgreasi
Copy link
Contributor

thgreasi commented Dec 4, 2016

Thanks for your contribution on this 👍

@yenoh2
Copy link
Contributor Author

yenoh2 commented Dec 5, 2016 via email

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.

3 participants