ngx-datatable
ngx-datatable copied to clipboard
Row Hover Event causing template bindings to be constantly recalculated
I'm submitting a ... (check one with "x")
[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, post on Stackoverflow or Gitter
Current behavior
As you hover over the rows of a rendered table - it seems a form of change detection is kicking in, causing all template functions to be called multiple times a second. This is causing a performance problem by causing a lot more work than necessary to occur on the front end of our app.
Expected behavior
I'd expect that hovering over the table rows would not trigger all template bindings to be updated, as no data would have changed.
Reproduction of the problem
Let's say you have a simple ngx column:
<ngx-datatable-column name="Company" width="300">
<ng-template let-value="value" ngx-datatable-cell-template>
{{ uppercase(value) }}
</ng-template>
</ngx-datatable-column>
With the corresponding component method for uppercase
being:
uppercase(word) {
console.log("processing the uppercase...")
return word.toUpperCase()
}
On the rendered table, as you hover over the rows, you'll see that the console log message is appearing multiple times per second - demonstrating multiple calls.
What is the motivation / use case for changing the behavior? The triggering of the function bindings, even if fast individually, being ran constantly without need is causing a performance issue, and the page to even feel janky.
Please tell us about your environment: Mac OSX, latest Chrome
- Table version: 10.2.3
Still an issue with the latest version (10.2.3). It appears the issue was brought in with 10.1.0
- with the Row Hover Event
feature. I've tested with 10.0.5
to confirm that the issue is not present there. I feel that this represents a performance regression, and for those who have desire to track hover events - it shouldn't be causing a 'digest loop' style recalculation when simply moving the mouse over table rows.
-
Angular version: 4.2.4 (and tested with 4.3.6)
-
Browser: Latest Chrome, Safari etc
-
Language: all
I experience similar issue with scrolling. I want to load data at once, calculate all bindings and not to trigger recalculation on every scroll.
@mrjamesriley changelog shows v10.1.0 adds activation via mouseenter
event in addition to mouse click and keyboard arrow keys that were already triggering the activate
event. This is by design now. See activate event.
@andrii-oleksyshyn Are you using scrollbarV
? If so, all bindings will need to be recalculated every scroll in order to virtualize the scroll view. As far as triggering the event on scroll, I did testing, and the mouseenter
event is only processed when the mouse moves irrespective of the scrolling. I did see some browser bugs in relation to scroll inertia but the overhead it creates isn't significant in my testing.
@mrjamesriley I did suggest a perf optimization where the event would be listened to outside of the Angular zone and only when there are listeners, i.e. attaching to the activate
event, would the event be triggered back inside the Angular zone resulting in change detection. That would be more of a pay for play model, even though in my testing I didn't see much a difference. All that would be moot if you already listen to the activate
event anyways. Would that help your performance woes?
Thanks for the response @wizarrc. In our case, we have no need for the activate of a row, especially on hover event. It really feels excessive to recalculate all bindings every time the mouse moves over a table - with it triggering many times a second. It causes the experience to quickly get janky. Is there not a way to disable this 'listener' - to opt out of the activate event so that it does not cause all bindings to be recalculated as frequently as it is doing to?
Ya, there is some changes we could make in terms of event delegation rather than individual bindings.
@mrjamesriley How are you measuring? I'm a bit perplexed. In my testing, it will only trigger once when entering a new cell, and that's it. My concerns were related to scrolling, and after testing, it turns out that it doesn't trigger an event if you are scrolling and do not move the mouse. You keep referring to a hover event. I'm not aware of such an event. The event I am referring to is mouseenter
event that might feel like a hover event. The event that I am referring to by its very nature, only triggers once per cell, think of it like a mouse click every time the mouse moves over a new cell.
That said, there are some ways that you could better handle that to make it even faster, but I'm not sure how you are noticing it. Also, I don't see how that's different than say a mouse click on a cell causing the same effect. There are so many other inefficient code that causes recalculation that this one seems small in comparison.
@amcdnl I proposed a solution to this perf issue here if you are interested.
@wizarrc @amcdnl
Hello, I'm still having problems here. The issue is as I describe in the initial post, but to hopefully clarify the issue. This is the problem I'm seeing:
- I have a table with 25 rows showing. A few of the columns are using fast-running functions to dynamically generate or modify the data that is displayed (e.g the
uppercase()
function in my original post). - As I scroll down the page, or simply move my mouse across the table - the 'change detection' is kicking in multiple times a second. The
mouseenter
event may only trigger once per cell, but as the mouse passes over multiple rows - this is being triggered multiple times. - This is causing all 25 rows to have their template functions re-ran multiple times a second, an unnecessary and undesired consequence.
Now in reality, I have multiple tables on a single page - and the impact of the hover triggering change detection is that every visible row, of every table on the page is having it's template functions re-ran. This is resulting in a page that suffers a performance hit every time the cursor passes over any row. Can you see how this is a problem?
Maybe I'm overlooking something here - but the desired result is that merely scrolling through a page (or moving the cursor around), should not have have the multiple tables constantly re-running their template functions (the unnecessary computation should be spared).
To clarify, by 'template functions', I'm referring to the uppercase()
function in my example code in the original post. By 'change detection' I'm referring to this function (in this example) being re-ran for all rows in all tables, as the cursor enters each row. Is it not possible to disable the hover/mouseenter event from kicking off the change detection?
Appreciate your time
@wizarrc @amcdnl
Hello!
Are there any updates on removing unnecessary change detection calls on events like mouseenter, when there's no subscribers to them? This is a big performance hit, when using [virtualization]="false", which is slow for MS Edge and mobile devices.
If you have 100 items, there would be 100 rowClass calls, 100 filter and methods call on a single mouseenter, which may quickly get very slow.
@mrjamesriley There is a quick workaround for intercepting mouseenter events and not passing them down to rows, like
window.addEventListener('mouseenter', function (event) {
event.stopPropagation();
}, true);
Other option is to use angular Pipes, they get cached and won't be called not even on a click event, if the data hasn't changed
I did a pr that will fix this issue for those who like me don't need mouseenter events in their app
Like @555ea mentioned intercepting mouseenter
event from a parent component or window during capturing phase and stopping it avoids executing the methods like rowClass
multiple times per each row.
But the workaround only solves part of the problem, any keydown
events when a row is focused still executes the rowClass
methods multiple times.
@ViewChild('datatable', { static: false }) public datatable: ElementRef;
public ngAfterViewInit() {
this.dataTable.nativeElement.addEventListener('mouseenter', this.onMouseEnter.bind(this), true);
}
public ngOnDestroy() {
this.dataTable.nativeElement.removeEventListener('mouseenter', this.onMouseEnter.bind(this));
}
private onMouseEnter(event: MouseEvent) {
event.stopPropagation();
}
Here's the StackBlitz for reproducing the issue.
@surya-pabbineedi This is still a major problem.