ui-sortable icon indicating copy to clipboard operation
ui-sortable copied to clipboard

Allow replace-transclude the sortable elements at link time

Open sszdh opened this issue 9 years ago • 13 comments

Code to reproduce issue:

app.js

var app = angular.module('App', ['ui.sortable']);

app.controller('MainCtrl', ['$scope', MainCtrl]);

function MainCtrl($scope) {
  $scope.items = [
    {id: 1, type: 'dog', name: 'galardo'},
    {id: 2, type: 'cat', name: 'loossi'},
    {id: 3, type: 'fish', name: 'sharkie'}
  ];
}

app.directive('myDirective', ['$compile', myDirective]);

function myDirective($compile) {
  return {
      restrict: 'EA',
      replace: true,
      //template: '<div ng-bind="item.name" item="item"></div>',
      scope: {
          type: "@",
          item: "="
      },
      link: link
  };

  function link(scope, element, attrs) {
      var view = $compile('<div ng-bind="item.name"></div>')(scope);
      element.replaceWith(view);
  }
}

index.html

<div ui-sortable ng-model="items">
    <my-directive item="item" ng-repeat="item in items"></my-directive>
 </div>

Console Error:

Error: [ngRepeat:dupes] Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: item in items, Duplicate key: undefined:undefined, Duplicate value: undefined
http://errors.angularjs.org/1.4.7/ngRepeat/dupes?p0=item%20in%20items&p1=undefined%3Aundefined&p2=undefined
    at http://localhost/angular-tests/lib/angular/angular.js:68:12

P.S: If we use template property instead of link function, there is no issue.

Cheers.

sszdh avatar Nov 08 '15 21:11 sszdh

Great finding!!! Does the problem get fixed if you add a track by id in your ng-repeat??? If yes, then the case might be that there is a digest loop where your directive isn't yet linked and the element hasn't yet been replaced, so since there is no "track by", angular uses the actual html for tracking, resulting dupes. Any chance you can make a codepen example of this?

thgreasi avatar Nov 08 '15 21:11 thgreasi

Hi, @thgreasi

Does the problem get fixed if you add a track by id in your ng-repeat???

No it doesn't. Using track by item.id throw same error but using track by $index throw no error but no sortable functionality (in some case only one item could be moved)!

Any chance you can make a codepen example of this?

Yes , here it is.

Actually this issue raised in Directive-in-Directive too.

Cheers

sszdh avatar Nov 09 '15 11:11 sszdh

See what happened in html source when we sorting one element intrack by $index mode:

<div ui-sortable="" ng-model="items" class="ng-pristine ng-untouched ng-valid ng-isolate-scope ui-sortable">
    <!-- ngRepeat: item in items track by $index -->
   <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>
    <!-- end ngRepeat: item in items track by $index -->
    <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>
    <!-- end ngRepeat: item in items track by $index -->
    <my-directive item="item" type="item.type" ng-repeat="item in items track by $index" class="ng-scope ng-isolate-scope"></my-directive>

   <!-- end ngRepeat: item in items track by $index -->
   <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">sharkie</div>
    <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">galardo</div>
    <div ng-bind="item.name" class="ng-binding ng-scope ui-sortable-handle">loossi</div>
</div>

sszdh avatar Nov 09 '15 12:11 sszdh

It seems that its a problem with the replacement of the original element... It's true that ui-sortable is a little too sansitive about manipulations on the repeated element.

I saw that you use a "type" attribute. If that's your main use case, have you seen the example in README (http://codepen.io/thgreasi/pen/uyHFC) regarding dynamic templates?

thgreasi avatar Nov 09 '15 12:11 thgreasi

@thgreasi Thanks, I already checked that.

Actually my main reason of using ng-model in directive is to track curriculum changes and then make an $http call to server!

But may I ask you how your tg-dynamic-directive can help me?

Actually I have a list like this:

var data = [
 {id: 1, type: 'chapter', title: 'chapter 1'},
 {id: 1, type: 'lecture', title: 'lecture 1-1'},
 {id: 2, type: 'lecture', title: 'lecture 1-2'},
 {id: 2, type: 'chapter', title: 'chapter 2'},
 ...
];

and a wrapper directive, something like that:

app.directive('curriculum', ['$compile', curriculum]);

function curriculum($compile) {
    return {
        restrict: 'EA',
        replace: true,
        scope: {
            type: "@",
            item: "="
        },
        link: link
    };

    function link(scope, element, attrs) {
        var view = $compile('<curriculum-'+ scope.type +' item="item"></curriculum-' + scope.type + '>')(scope);
        element.replaceWith(view);
    }
}

app.directive('curriculum-chapter', [curriculumChapter]);
function curriculumChapter() {
 return {
    ...
    template: '/templates/chapter.tpl.html'
    ...
 }
}

app.directive('curriculum-lecture', [curriculumLecture]);
function curriculumLecture() {
return {
    ...
    template: '/templates/lecture.tpl.html'
    ...
 }
}

So is it possible to use ng-ui-sortable in this case?

Thanks in advance.

sszdh avatar Nov 09 '15 13:11 sszdh

Unfortunate tg-dynamic-directive does not replace its nested directives tag (<tg-dynamic-directive> & <div ng-include>) and it makes my HTML structure complicated and my CSS selectors does not work properly!

I thinks angularjs should allows nested directive with template: ..., replace: true options in both!

sszdh avatar Nov 09 '15 14:11 sszdh

With the tg-dynamic-directive you could avoid creating a directive that would manually compile the appropriate directive based on the type parameter. You would only need to define a function that retrieves the template to be included based on the type provided.

thgreasi avatar Nov 09 '15 15:11 thgreasi

As you saw transclusions with replace: true can cause problems. If the suggested solution described in README works for you, then you should focus on fixing your CSS. I think this would be easier than solving the "replace: true & dynamic $compiling & linking" problem.

thgreasi avatar Nov 09 '15 15:11 thgreasi

My experience on a similar implementation convinced me that ng-include was the way to go for this use case, since I experience similar problems with "replaced transclusion" as well. That was the reason I created tg-dynamic-directive, so that others wouldn't have to mess with the same problem from scratch. If you feel it is too restrictive, then just copy its implementation, customize it (probably by adding some css classes to the elements) and integrate it to your directives.

On Mon, Nov 9, 2015, 16:25 Soheil Samadzadeh [email protected] wrote:

Unfortunate tg-dynamic-directive does not replace its nested directives tag ( &

) and it makes my HTML structure complicated and my CSS selectors does not work properly!

I thinks angularjs should allows nested directive with template: ..., replace: true options in both!

— Reply to this email directly or view it on GitHub https://github.com/angular-ui/ui-sortable/issues/405#issuecomment-155077681 .

Thodoris Greasidis Computer, Networking & Communications Engineer

thgreasi avatar Nov 09 '15 16:11 thgreasi

@thgreasi, this was a good discussion. and thank you.

Finally I ended up with this solution:

app.directive('curriculum', curriculum);

function curriculum() {
    return {
        restrict: 'EA',
        replace: true,
        template: '<div ng-include="templateUrl" include-replace></div>',
        link: link,
        scope: {
            type: '@',
            item: '='
        }
    };

    function link(scope) {
        scope.templateUrl = "/templates/curriculum-" + scope.type + ".tpl";
    }
}


app.directive('includeReplace', curriculum);

function includeReplace() {
    return {
        require: 'ngInclude',
        replace: true,
        restrict: 'A',
        link: function (scope, el) {
            el.replaceWith(el.children());
        }
    };
}
<curriculum ng-repeat="item in curriculum" type="[[item.type]]" item="item" ></curriculum>

It may be good solution for cases that only depends on templates. But It prevents me to encapsulate directive-specific logic (such as controller, etc) into each directive! So Sad!

And I think we should get back and focus on main issue of ng-ui-sortable mentioned above "Sort-able directive with link function problem" and trying to fix it.

Thank you very much

sszdh avatar Nov 09 '15 23:11 sszdh

I'm glad that it worked for you :+1: That's more or less the solution that I was suggesting and that I followed in my case as well. That's also what inspired me for the implementation of tg-dynamic-directive. Unfortunately you can't directly define controllers or directives, but obviously you can do that inside your included template.

thgreasi avatar Nov 09 '15 23:11 thgreasi

I also changed the title to what I think better describes the problem. My opinion is that is not the linking that is causing the problem (since your directive and tg-dynamic-directive also have link functions) but the fact that we are replacing the sortable items that are generated by the ng-repeat.

thgreasi avatar Nov 09 '15 23:11 thgreasi

:thumbsup:

sszdh avatar Nov 09 '15 23:11 sszdh