fix(positioning): cache management #3214
Conversation
There was a problem hiding this comment.
This is a good idea and could indeed improve performances for some use cases, but a properly designed Angular application shouldn't face performance issues related to this piece of code. The latter is executed after every change detection, and change detection cycles should not occur too often.
I would be in favor of such a solution if it would not have to make some strong hypotheses to keep the benefits of the optimization. More precisely, a proper caching solution should take into account the positions and sizes of the host and target elements, and that would still require some expensive calculations from the rendering side.
Therefore I would consider this as not needed
Codecov Report
@@ Coverage Diff @@
## master #3214 +/- ##
=========================================
+ Coverage 90.77% 92.1% +1.32%
=========================================
Files 94 92 -2
Lines 2711 3102 +391
Branches 507 517 +10
=========================================
+ Hits 2461 2857 +396
+ Misses 190 179 -11
- Partials 60 66 +6
Continue to review full report at Codecov.
|
fbasso
force-pushed
the
position-cache
branch
from
772f3c1
to
fe2d6ac
Compare
|
This PR is not valid anymore, as now positioning is done with popperjs |
The purpose of this PR is to increase performance of re-positioning popup by caching the popup dimension values.
It can partially fixed #3199
This is not intended to be straightly merged, as we need to investigate the implications. For example when the popup content changes while it's open (for example, the typeahead).