sinequa / sba-angular

Sinequa's Angular-based Search Based Application (SBA) Framework
https://sinequa.github.io/sba-angular/
MIT License
30 stars 23 forks source link

Performances issues caused by HostListener on facet-card #106

Closed Guillaume-Developer closed 6 months ago

Guillaume-Developer commented 1 year ago

When many facet-card are used on the screen, some of the other click events on the screen are being delayed by the @HostListener('window:click', ['$event']). Since this part of the code is only used conditionally, it might be good to initialize it only in the case where collapseOnClickOutside == true, using addEventListener instead of HostListener.

ericleib commented 1 year ago

Thank you for reporting it. I understand that you tried to remove the HostListener, and it had a noticeable effect on performance? Additionally, what is the context in which you have a lot of facets displayed on the screen ? We have a lot of them in the usage-analytics but the performance seems to be ok. Note that we made many simplifications to the facet components in recent versions (currently in nightly builds), which should generally improve performance and UX. I'd be interested in your feedback if you have the opportunity to give it a try.

Guillaume-Developer commented 1 year ago

I tested by removing the HostListener and yes, performances improved a lot (same as when the facets are deleted from the screen). The customer wants to have a "lot" of filters on the left side, so we have about 9 of them. The multi-facets was judged by the customer as not very user-friendly and was replaced in the early stages of development by multiple facets. I saw the simplifications and greatly welcome them! I had actually performed a short simplifications of the actions as well on my side, because it was impacting the performances.

To be more precise on the performances impact, it targets components that are creating a dropdown like menu/popup. The creation of the dropdown is waiting for all the HostListener events to be checked before actually being constructed, which gives me a delay of about 2 seconds between the click and the apparition of the popup/menu. I am using the ng-bootstrap library (NgbDropdown directive) for these specific dropdown, as the Action component did not fit the need at the time. The NgbDropdown directive also has a global listener on click events, for notifying the dropdown to close on outside clicks, but this does not impact the performances as the component, and the listener, are only created when the dropdown is opened.

I believe I also saw an impact on the performances of normal click events that would make other elements appear or disappear from the screen (for example the preview), but I would need to re-confirm that.

Hope this clarifies!

ericleib commented 1 year ago

A 2s delay sounds enormous! And I am surprised that it is caused by the HostListener of the facets because the click handler is not doing much. Does this affect all dropdown menus (in the navbar, in the facet headers, etc.) ? Or just the dropdown menus in which you use NgbDropdown? Do you use a combination of both in your app?

Note that it is possible to create dropdown menus without all the stuff from the action module AND without NgbDropdown. We have a directive activated by data-bs-toggle="dropdown" that works magically with code that follows the standard Bootstrap syntax (https://getbootstrap.com/docs/5.2/components/dropdowns/)

Anyway, I created a ticket and implemented your proposal (add the document listener conditionally).

Guillaume-Developer commented 1 year ago

Thank you for the ticket and evaluation of my proposal!

I'll have a look at the data-bs-toggle="dropdown" when I have time, see if I can implement it where we used the ng-bootstrap dropdown. I think it might be better to not be dependent on the ng-bootstrap library in the future so it might actually help us, thanks.

hebus commented 6 months ago

in the future we will use pure css dropdown behavior