Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ionicModal performance issue after a collection-repeat #2935

Closed
rickyk586 opened this issue Jan 14, 2015 · 12 comments · Fixed by #2964
Closed

ionicModal performance issue after a collection-repeat #2935

rickyk586 opened this issue Jan 14, 2015 · 12 comments · Fixed by #2964

Comments

@rickyk586
Copy link

A collection repeat listens for a resize event and does a heavy recalculation when it is fired. When a modal is shown, it triggers this resize event:

ionic.trigger('resize');

Because this is a heavy operation, it has performance impacts. The modal is not shown right away because each collection-repeat in the app has to go through it's resize recalculation.

I was able to temporarily fix this by commenting out that line in ionicModal.show(). It did not seem to have any negative side effects.

A possible improvement could be to only have the collection-repeat listen for this event when its view is currently active. That way a modal on another view will not be affected by it (and every other collection-repeat that has been shown in the app).

@pulakb
Copy link

pulakb commented Jan 14, 2015

@rickyk586 can you please share a codepen?

@iver56
Copy link

iver56 commented Jan 16, 2015

This issue becomes serious when there are many items (f.ex. 1000) in the list(s) that use(s) collection-repeat. It can freeze the app for several (5-15) seconds, depending on how fast the device is. See also what I posted about the issue here:
#1912 (comment)

@panurge-ws
Copy link

I can confirm that the issue of the freezing/stuck of the Modal is related to collection-repeat and 'resize' event.
Commenting out the line from $ionicModal.show that trigger 'resize' event, fixes the problem.

@panurge-ws
Copy link

The same issue related to performance for collection-repeat is occurring also each time the keyboard is shown on Android, causing the app freezing for several seconds. (I have about 500 items in the collection-repeat model).

@stianjensen
Copy link

I guess this problem was introduced with the cached views in beta 14. Moving the binding of the resize event listener in collection-repeat to enter and leave events, should probably fix this. No reason for a collection-repeat to listen for resize events when it's not visible, right?

@panurge-ws
Copy link

@stianjensen yes, you are right.
I have fixed that in this way, inside the collection-repeat directive, but I'm not sure if this can affect some other behaviors. Anyway, I had some great improvements in terms of performances. But the problem related to this issue is that the ionicModal triggers the resize event when showing, causing the collection-repeat to re-render the list, so it's not related to the caching system.

  // panurge
  var requiresRerender, pauseRender = false;
  function rerenderOnResize() {
    // panurge
    if (pauseRender){
      return;
    }
    rerender(listExprParsed($scope));
    requiresRerender = (!scrollViewContent.clientWidth && !scrollViewContent.clientHeight);
  }

  function viewEnter() {
    if (requiresRerender) {
      rerenderOnResize();
    }
    pauseRender = false;
  }

  // panurge
  function viewLeave() {
    pauseRender = true;
  }

  scrollCtrl.$element.on('scroll.resize', rerenderOnResize);
  ionic.on('resize', rerenderOnResize, window);
  var deregisterViewListener;
  var deregisterViewListenerLeave;
  if (navViewCtrl) {
    deregisterViewListener = navViewCtrl.scope.$on('$ionicView.afterEnter', viewEnter);
    // panurge
    deregisterViewListenerLeave = navViewCtrl.scope.$on('$ionicView.afterLeave', viewLeave);
  }

  $scope.$on('$destroy', function() {
    collectionRepeatManager.destroy();
    dataSource.destroy();
    ionic.off('resize', rerenderOnResize, window);
    (deregisterViewListener || angular.noop)();
    // panurge
    (deregisterViewListenerLeave || angular.noop)();
  });

@felquis
Copy link

felquis commented Feb 5, 2015

That's it, my app is REALLY slow because of it, when I open a modal it's just like hell!

Please, consider this issue <3

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2015

Checking this out.

@ajoslin ajoslin closed this as completed in b7a0968 Feb 9, 2015
@felquis
Copy link

felquis commented Feb 9, 2015

Nice! I want to test it, does Ionitron has some delay to apply the last commit to ionic-bower?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2015

It should be up now.

@felquis
Copy link

felquis commented Feb 9, 2015

I can confirm that the issues #2890 is completely fixed in v1.0.0-beta.14-nightly-1011 👌

Much better now, congratulations @ajoslin 😄

@panurge-ws
Copy link

With current nightly, it's not fixed when the collection-repeat is in a tab different from the one that opens the modal.
http://codepen.io/anon/pen/vERyVx

Switch to tab "Modal" click the button, the first time you open the modal, it's very slow.
Switch to ng-repeat and the first time the show transition is smooth.
If you comment the ionic.trigger('resize') in the $ionicModal factory, also with the colleciton-repeat the show action is smooth.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 6, 2018
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 a pull request may close this issue.

8 participants