Bug: Plugin is calling changeDetection on every mousemove

[antonbuks]

Reproduction: https://stackblitz.com/edit/ngx-drag-to-select-2bpbcb

If you check the console you will see that every time the mouse moves, the test function is called as a result of change detection. This can be easily fixed by running the plugin outside angular to avoid change detection from eventlisteners.

Event listener:

const mousemove$ = fromEvent<MouseEvent>(window, 'mousemove').pipe( filter(() => !this.disabled), share() );

ngx-drag-to-select/projects/ngx-drag-to-select/src/lib/select-container.component.ts

Line 157 in c47dee4

const mousemove$ = fromEvent<MouseEvent>(window, 'mousemove').pipe(

[d3lm]

So CD actually has to be triggered when the mouse moves, because otherwise the select box won't be re-rendered on the screen and it becomes janky if I audit / throttle it. Curious to hear your thoughts tho.

[antonbuks]

Ok I see, I haven't had a lot of time this week to look at all the code but I'll have a check this weekend and get back to you.

If you can tell me more precisely how change detection affects re-rendering (point me to some code) that would make it faster.

[bertrandg]

A nice optimisation for performance would be to subscribe on mousemove only between mousedown and mouseup.
Anyway, thanks for your library ;)

[d3lm]

Good point @bertrandg! I like that. But as I said, we won't be able to run this outside of Angular zone because otherwise the UI won't be updated properly and the selection box won't be re-drawn and throttling it will make it janky. If I remember correctly this is something I tried when I initially built this lib.

[bertrandg]

Yeah I understand.

Subscribing on mousemove only between mousedown and mouseup is already a big win for the library.

But to go deeper, it would be possible to track all mouse events outside zone.
Here is what is needed inside SelectContainerComponent:

• Call changeDetectorRef.detach() in constructor.
• Call changeDetectorRef.detectChanges() when need to refresh select box view (refresh only component view, not whole app).
  [Alternative option is to use directly Renderer2.addClass() / removeClass() / setStyle() / removeStyle() on #selectBox]
• Wrap all EventEmitter.emit() inside a NgZone.run(() => {...})

[d3lm]

Yep, sounds reasonable. I ll have a look and see which solution I ll go with. Maybe deteching the whole container from the CD cycle might be a good idea. This way I can call CD manually when needed.

[Savers80]

Hi,
i'm novice with Angular, but it's seems naive solution to solve cd on whole app could be:
1- in class KeyboardEventsService
public htmlElement: any = window;
constructor(@Inject(PLATFORM_ID) private platformId: Object) {
if (isPlatformBrowser(this.platformId)) {
this._initializeKeyboardStreams();
}
}
public initializeKeyboardStreams() {
if (isPlatformBrowser(this.platformId)) {
this._initializeKeyboardStreams();
}
}
and change window references to this.htmlElement

2 - SelectContainerComponentin class use :
ngOnInit() {
this.keyboardEvents.htmlElement = this.hostElementRef.nativeElement;
this.keyboardEvents.initializeKeyboardStreams();
}

But this don't solve problem about mouseevents on container triggers CD

I will study EventManagerPlugin to run outsideAngularZone and how integrate it in project, but it is a little hard for me now

[Saonela]

Any news on this ?

[d3lm]

Hey @Saonela, my apologies but I haven't had the time yet to delve into this. I am a bit swamped at the moment but if you want to take a stab at it I am more than happy to support you. I would appreciate a PR. First we should upgrade the library to v9 tho. That is also on my todo list.