Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Performance issue with tooltips #1450

Closed
vincentbernat opened this issue Dec 23, 2013 · 13 comments
Closed

Performance issue with tooltips #1450

vincentbernat opened this issue Dec 23, 2013 · 13 comments

Comments

@vincentbernat
Copy link

I am using a lot of tooltips. Something like several thousands. Since the template for the tooltip is compiled for each of them, the page is quite slow to render and spent a lot of time in parsing HTML. Would it be possible to compute tooltip template later in the life-cycle? For example, only when the tooltip has to be displayed.

Thanks.

@pkozlowski-opensource
Copy link
Member

I guess this is something we should look into, yes. Having said this an AngularJS application with several thousands bindings will hit a limit somewhere else, I guess....

@vincentbernat
Copy link
Author

As a first workaround, here is how I bundled the original tooltip into another directive to get this "late" evaluation:

reportResultsApp.directive(
    "rrTooltip", [ "$compile", function($compile) {
        return {
            restrict: 'A',
            scope: {
                content: '@rrTooltip'
            },
            compile: function($element, attr) {
                return function(scope, element, attr) {
                    element.on("mouseenter", function(event) {
                        // Only now, we will build the new element
                        // with the tooltip if it doesn't exist (or
                        // doesn't match the tooltip)
                        var inside = element.children();
                        if (!inside.length) {
                            inside = angular.element("<span>");
                        }
                        if (inside.attr("tooltip") === scope.content) return;
                        inside.attr("tooltip", scope.content);

                        // And insert it
                        var compiled = $compile(inside);
                        var linked = compiled(scope);
                        element.empty();
                        element.append(linked);
                    });
                }
            }
        }
    }
]);

This gives me about the same performance as with no tooltip. However, my bindings are not updated, so maybe it would crumble when this happens.

@chrisirhc
Copy link
Contributor

I haven't tested this thoroughly so I might be wrong but my feeling is that it's actually the linking that's causing the slowdown. Right now both compilation and linking of the template is done in the link function of the $tooltip.
Here's what I propose (I'll send a PR in a bit):

  1. Compile the template during the compile phase
  2. Link the template only on demand, such as during mouseenter
  3. Destroy the clone/scope on mouseleave

This will also solve #1191 and should speed things up considerably as none of the watchers on the template will actually be on the scope.

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 23, 2013
@chrisirhc
Copy link
Contributor

@vincentbernat Still a work-in-progress but you can check out https://github.com/chrisirhc/angular-ui-bootstrap/tree/feature/tooltip-scope-rework to see if that resolves the performance issue you're running into.

@vincentbernat
Copy link
Author

@chrisirhc I tried but I get an $injector:modulerr when replacing the current version of angular-ui-bootstrap with yours. However, this is unrelated to your change: I still get this error if I compile myself the 0.7.0 version. A diff with the version from gh-pages show me that modal/backdrop.html and modal/window.html templates are missing from my version. Any idea what the problem could be?

@chrisirhc
Copy link
Contributor

@vincentbernat Try running grunt instead of running grunt build.

@vincentbernat
Copy link
Author

@chrisirhc This worked. Do you think I should file a bug/PR for this (task html2js:dist should also be in build target)?

Otherwise, performance is pretty good! Currently, the tooltip disappear after a second without the mouse leaving the element.

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 24, 2013
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 24, 2013
@chrisirhc
Copy link
Contributor

@vincentbernat That's good. The positions are broken. I'll fix that in a bit.

Regarding the html2js task, I think it's a known issue but feel free to file it or submit a pull request for that.

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 24, 2013
- Calling $digest is enough as we only need to digest the watchers in
  this scope and its children. No need to call $apply.

- No need to test for cached reference when tooltip isn't visible as
  the tooltip has no scope.

Attempts to fix angular-ui#1450
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 24, 2013
- Calling $digest is enough as we only need to digest the watchers in
  this scope and its children. No need to call $apply.

- No need to test for cached reference when tooltip isn't visible as
  the tooltip has no scope.

Fixes angular-ui#1450 and angular-ui#1191
@vincentbernat
Copy link
Author

The html2js stuff has already been discussed in #253.

@chrisirhc
Copy link
Contributor

@vincentbernat , I've updated #1455 and fixed the position, tooltips should behave as expected now. Could you try it out and let me know what you think?

@vincentbernat
Copy link
Author

@chrisirhc , it works fine for me. Performance is good and I didn't trigger any issue. Thanks!

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 28, 2013
- Calling $digest is enough as we only need to digest the watchers in
  this scope and its children. No need to call $apply.

- Set invokeApply to false on $timeout for popUpDelay

- No need to test for cached reference when tooltip isn't visible as
  the tooltip has no scope.

Fixes angular-ui#1450 and angular-ui#1191
@pkozlowski-opensource
Copy link
Member

I've pushed 79d2627 by accident, had to revert it...

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Dec 30, 2013
- Calling $digest is enough as we only need to digest the watchers in
  this scope and its children. No need to call $apply.

- Set invokeApply to false on $timeout for popUpDelay

- No need to test for cached reference when tooltip isn't visible as
  the tooltip has no scope.

Fixes angular-ui#1450 and angular-ui#1191
@trivedigaurav
Copy link

I am still seeing performance issues with tooltips.

I have about 500 of them on my page. And they cause the page to become unresponsive for several seconds on load.

Sometimes it even causes my Firefox browser to show the unresponsive script prompt. Although everything works fine after that initial wait.

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

Successfully merging a pull request may close this issue.

4 participants