Famous / famous-angular

Bring structure to your Famo.us apps with the power of AngularJS. Famo.us/Angular integrates seamlessly with existing Angular and Famo.us apps.
https://famo.us/angular
Mozilla Public License 2.0
1.92k stars 275 forks source link

Error when removing element in combination with pipe-to #185

Closed janober closed 9 years ago

janober commented 10 years ago

Sorry have again a pipeing issue.

When I diplay multiple items with ng-repeat and then remove one, I get the following error message:

TypeError: undefined is not a function
    at bulkUpdatePipes (http://127.0.0.1/famous-angular-examples/app/bower_components/famous-angular/dist/famous-angular.js:732:31)
    at unpipesFromTargets (http://127.0.0.1/famous-angular-examples/app/bower_components/famous-angular/dist/famous-angular.js:748:7)
    at http://127.0.0.1/famous-angular-examples/app/bower_components/famous-angular/dist/famous-angular.js:4202:27
    at Scope.$broadcast (http://127.0.0.1/famous-angular-examples/app/bower_components/angular/angular.js:12937:28)
    at Scope.$destroy (http://127.0.0.1/famous-angular-examples/app/bower_components/angular/angular.js:12595:14)
    at ngRepeatAction (http://127.0.0.1/famous-angular-examples/app/bower_components/angular/angular.js:20493:27)
...

This only happens when I use "fa-pipe-to" on the draggable. I tried to unpipe manually before removing the item but got the same. But not sure if I did it correctly. Would be great to get a tipp how to avoid that and implement that correctly. Thanks!

The example bellow is on the basis of the draggable famous-angular-example.

JavaScript Code:

angular.module('integrationApp')
  .controller('TestCtrl', function ($scope, $famous) {
    var EventHandler = $famous['famous/core/EventHandler'];

    var data = [];
    data.push( {id: 0, name: 'zero', position: [10, 100], handler: new EventHandler() } );
    data.push( {id: 1, name: 'one',  position: [10, 200], handler: new EventHandler() } );
    data.push( {id: 2, name: 'two',  position: [10, 300], handler: new EventHandler() } );

    $scope.nodes = data;

    $scope.remove = function() {
      delete $scope.nodes[1];
      console.log($scope.nodes);
    }

  });

HTML Code:

<fa-app style="width: 320px; height: 568px; overflow: hidden;">
    <fa-modifier ng-repeat="node in nodes"  fa-translate="[10,500]" fa-size="[80, 40]">
        <fa-surface>
          <button ng-click="remove()">Remove one</button>
      </fa-surface>
    </fa-modifier>

  <fa-modifier ng-repeat="node in nodes"  fa-translate="node.position" fa-size="[80, 40]">
    <fa-draggable fa-pipe-from="node.handler" fa-pipe-to="node.handler">
      <fa-surface fa-background-color="'red'" fa-size="[undefined, undefined]" fa-pipe-to="node.handler">
        {{node.id}}: {{node.name}}
      </fa-surface>
    </fa-draggable>
  </fa-modifier>

</fa-app>
janober commented 10 years ago

Anybody anything?

zackbrown commented 10 years ago

Hey @janober —

Thanks for your patience on this. I just tried out your code and there were a couple of issues:

  1. Calling delete $scope.nodes[1]; was doing some funky stuff to your array. It was still being considered an array of length 3, though it was missing the key 1 (only had 0 and 2.) Angular was still ng-repeating 3 items, but the second one was undefined. Rather than tangle with this, I would recommend using $scope.nodes.splice(1, 0), which will remove from the array (in place) one element at index 1 and correctly updates the array to know that its new length is two, updating subsequent indexes appropriately.
  2. There's no need to use fa-pipe-to on an fa-draggable; in fact, it's not supported. Generally what you want to do with fa-pipe-to and fa-pipe-from in this case is grab event input from the surface (only Surfaces gather user input, as they contain and channel normal DOM and DOM events) and passing it onto the draggable. The way that's done in F/A is to use fa-pipe-to on the fa-surface to pipe your Surface's events TO some event handler, and then fa-pipe-from on the fa-draggable to then pipe those same events FROM the event handler to the draggable. Since the draggable itself isn't emitting additional events, it has no use for an fa-pipe-to. TL;DR remove the fa-pipe-to from your fa-draggable.

Here's a modified version of your code that works for me:

<fa-app style="width: 320px; height: 568px; overflow: hidden;">
    <fa-modifier ng-repeat="node in nodes"  fa-translate="[10,500]" fa-size="[80, 40]">
        <fa-surface>
          <button ng-click="remove()">Remove one</button>
      </fa-surface>
    </fa-modifier>

  <fa-modifier ng-repeat="node in nodes"  fa-translate="node.position" fa-size="[80, 40]">
    <fa-draggable fa-pipe-from="node.handler">
      <fa-surface fa-background-color="'red'" fa-size="[undefined, undefined]" fa-pipe-to="node.handler">
        {{node.id}}: {{node.name}}
      </fa-surface>
    </fa-draggable>
  </fa-modifier>

</fa-app>

Controller:

angular.module('integrationApp')
  .controller('TestCtrl', function ($scope, $famous) {
    var EventHandler = $famous['famous/core/EventHandler'];

    var data = [];
    data.push( {id: 0, name: 'zero', position: [10, 100], handler: new EventHandler() } );
    data.push( {id: 1, name: 'one',  position: [10, 200], handler: new EventHandler() } );
    data.push( {id: 2, name: 'two',  position: [10, 300], handler: new EventHandler() } );

    $scope.nodes = data;
    console.log($scope.nodes);

    $scope.remove = function() {
      $scope.nodes.splice(0, 1);
      console.log($scope.nodes);
    }
  });
janober commented 10 years ago

Hi Zack,

Thanks for the answer. Sadly does not work for me. The reason is that the above was just a simplified version. I am actually using an object not an array. But I just tested also with an array and because I had the same issue with both of them I thought it does not really matter which I use.

About the pipe-to. The problem is that I sadly need the pipe-to on the draggable because I need the "update" event from the draggable. So simply removing it does not work for me. Is related to the old issue #172 I had.

SuPenguin commented 10 years ago

Hello Jan,

I had the same issue when $destroy is called on a Draggable, from deleting an item in a ng-repeat or from ui-router. It appears that it goes through $destroy without testing if it's a modifier so it tries to renderNode.unpipe(EventHandler). I've corrected this by testing it again before updating the pipes on https://github.com/SuPenguin/famous-angular/commit/c7f19cd9e4ae904f1c6c34d848c91c49da24f0e6.

It may be a better way of doing this, i'll submit a pull request if i find one.

pencilcheck commented 9 years ago

Same issue, however I think fa-pipe-to on fa-draggable works for me, it is emitting the correct events.

@zackbrown Also I think $scope.nodes.splice(1, 0) doesn't remove the element since the second argument is 0. Are you sure this is the right?

pencilcheck commented 9 years ago

@SuPenguin your patch works, can you submit as a push request?

SuPenguin commented 9 years ago

Yes : https://github.com/Famous/famous-angular/pull/279

pencilcheck commented 9 years ago

Awesome, thanks

jordanpapaleo commented 9 years ago

@janober Is this still a valid issue? Thanks - Jordan

janober commented 9 years ago

Very sorry. Was working on another project the last months, so did not really check back here. I am actually just starting with this now again.

First, thanks a lot for everybody who worked on it! I just tested and it works fine now. So close it.