From 989776f9c05da9518ca2fd978e70a3fd16af14b0 Mon Sep 17 00:00:00 2001
From: danielwiehl <daniel.wiehl@yahoo.com>
Date: Sat, 26 Oct 2024 09:19:56 +0200
Subject: [PATCH] perf(components/viewport): avoid change detection cycle when
 scrolling the viewport

BREAKING CHANGE: Changed viewport to emit scroll events outside the Angular zone.

To handle scroll events inside the Angular zone, e.g., if updating component bindings, manually synchronize with the Angular zone, as follows:

```ts
inject(NgZone).run(() => {
  ...
});
```
---
 docs/site/tools/viewport.md                   |  4 +-
 .../viewport/src/viewport.component.html      |  3 +-
 .../viewport/src/viewport.component.spec.ts   | 46 ++++++++++++++++++-
 .../viewport/src/viewport.component.ts        | 35 ++++++++++----
 4 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/docs/site/tools/viewport.md b/docs/site/tools/viewport.md
index b87780c5..0762796c 100644
--- a/docs/site/tools/viewport.md
+++ b/docs/site/tools/viewport.md
@@ -78,12 +78,12 @@ The viewport component displays scrollbars when its content overflows. Scrollbar
   
 #### Inputs:
 - **scrollbarStyle**\
-  Controls whether to use native scrollbars or, which is by default, emulated scrollbars that sit on top of the viewport client. In the latter, the viewport client remains natively scrollable.\
+  Controls if to use the native scrollbar or a scrollbar that sits on top of the viewport. Defaults to `on-top`.
   Supported values are `native`, `on-top`, or `hidden`.
 
 #### Events:
 - **scroll**\
-  Emits upon a scroll event.
+  Emits when the viewport is scrolled. The event is emitted outside the Angular zone to avoid unnecessary change detection cycles.
 
 </details>
 
diff --git a/projects/scion/components/viewport/src/viewport.component.html b/projects/scion/components/viewport/src/viewport.component.html
index 86fe08c8..6586bfb8 100644
--- a/projects/scion/components/viewport/src/viewport.component.html
+++ b/projects/scion/components/viewport/src/viewport.component.html
@@ -3,8 +3,7 @@
      class="viewport"
      tabindex="-1"
      sciScrollable [sciScrollableDisplayNativeScrollbar]="scrollbarStyle() === 'native'"
-     cdkScrollable
-     (scroll)="onScroll($event)">
+     cdkScrollable>
   <div #viewport_client class="viewport-client" part="content">
     <slot></slot>
   </div>
diff --git a/projects/scion/components/viewport/src/viewport.component.spec.ts b/projects/scion/components/viewport/src/viewport.component.spec.ts
index d71fc64d..c8f23be5 100644
--- a/projects/scion/components/viewport/src/viewport.component.spec.ts
+++ b/projects/scion/components/viewport/src/viewport.component.spec.ts
@@ -9,7 +9,7 @@
  */
 
 import {ComponentFixture, TestBed} from '@angular/core/testing';
-import {Component, ElementRef, HostBinding, Input, Renderer2, ViewChild} from '@angular/core';
+import {Component, ElementRef, HostBinding, Input, NgZone, Renderer2, viewChild, ViewChild} from '@angular/core';
 import {By} from '@angular/platform-browser';
 import {Dictionary} from '@scion/toolkit/util';
 import {SciViewportComponent} from './viewport.component';
@@ -943,6 +943,50 @@ describe('Viewport', () => {
     fromDimensionSubscription.unsubscribe();
   });
 
+  it('should emit scroll events outside the Angular zone', async () => {
+    @Component({
+      selector: 'spec-viewport',
+      template: `
+        <sci-viewport (scroll)="onScroll()">
+          <div class="content">Content</div>
+        </sci-viewport>`,
+      styles: `
+        :host {
+          display: grid;
+          border: 1px solid black;
+          width: 300px;
+          height: 200px;
+
+          > sci-viewport > div.content {
+            height: 1000px;
+            background-color: lightblue;
+          }
+        }`,
+      imports: [SciViewportComponent],
+    })
+    class SpecComponent {
+
+      public viewport = viewChild.required(SciViewportComponent);
+      public scrolledInsideAngular: boolean | undefined = undefined;
+
+      protected onScroll(): void {
+        this.scrolledInsideAngular = NgZone.isInAngularZone();
+      }
+    }
+
+    const fixture = TestBed.createComponent(SpecComponent);
+    fixture.autoDetectChanges(true);
+    const testee = fixture.componentInstance;
+    const viewport = testee.viewport();
+
+    // Scroll the viewport.
+    viewport.scrollTop = 100;
+    await flushChanges(fixture);
+
+    // Expect scroll event to be received outside Angular.
+    expect(testee.scrolledInsideAngular).toBeFalse();
+  });
+
   describe('computeOffset', () => {
 
     it('should compute offset of element inside viewport', async () => {
diff --git a/projects/scion/components/viewport/src/viewport.component.ts b/projects/scion/components/viewport/src/viewport.component.ts
index 65b68721..3dd53ac1 100644
--- a/projects/scion/components/viewport/src/viewport.component.ts
+++ b/projects/scion/components/viewport/src/viewport.component.ts
@@ -8,12 +8,14 @@
  *  SPDX-License-Identifier: EPL-2.0
  */
 
-import {Component, ElementRef, HostListener, inject, input, output, viewChild, ViewEncapsulation} from '@angular/core';
+import {Component, effect, ElementRef, HostListener, inject, input, NgZone, output, untracked, viewChild, ViewEncapsulation} from '@angular/core';
 import {SciNativeScrollbarTrackSizeProvider} from './native-scrollbar-track-size-provider.service';
 import {coerceElement} from '@angular/cdk/coercion';
 import {SciScrollableDirective} from './scrollable.directive';
 import {SciScrollbarComponent} from './scrollbar/scrollbar.component';
 import {ScrollingModule} from '@angular/cdk/scrolling';
+import {fromEvent} from 'rxjs';
+import {subscribeIn} from '@scion/toolkit/operators';
 
 /**
  * Represents a viewport with slotted content (`<ng-content>`) used as scrollable content. By default, content is added to a CSS grid layout.
@@ -101,27 +103,24 @@ export class SciViewportComponent {
   protected nativeScrollbarTrackSizeProvider = inject(SciNativeScrollbarTrackSizeProvider);
 
   /**
-   * Controls whether to use native scrollbars or, which is by default, emulated scrollbars that sit on top of the viewport client.
-   * In the latter, the viewport client remains natively scrollable.
+   * Controls if to use the native scrollbar or a scrollbar that sits on top of the viewport. Defaults to `on-top`.
    */
   public scrollbarStyle = input<ScrollbarStyle>('on-top');
 
   /**
-   * Emits upon a scroll event.
-   *
-   * You can add [sciDimension] directive to the viewport or viewport client to be notified about layout changes.
+   * Emits when the viewport is scrolled. The event is emitted outside the Angular zone to avoid unnecessary change detection cycles.
    */
   public scroll = output<Event>();
 
+  constructor() {
+    this.installScrollEmitter();
+  }
+
   @HostListener('focus')
   public focus(): void { // do not rename to expose the same focus method like `HTMLElement.focus()`.
     this.viewportElement.focus();
   }
 
-  protected onScroll(event: Event): void {
-    this.scroll.emit(event);
-  }
-
   /**
    * Returns the number of pixels that the viewport client is scrolled vertically.
    *
@@ -279,6 +278,22 @@ export class SciViewportComponent {
 
     return offset;
   }
+
+  /**
+   * Emits when the scroll position changes.
+   */
+  private installScrollEmitter(): void {
+    const zone = inject(NgZone);
+    effect(onCleanup => {
+      const viewport = this._viewport();
+      untracked(() => {
+        const subscription = fromEvent(viewport.nativeElement, 'scroll')
+          .pipe(subscribeIn(fn => zone.runOutsideAngular(fn)))
+          .subscribe(event => this.scroll.emit(event));
+        onCleanup(() => subscription.unsubscribe());
+      });
+    });
+  }
 }
 
 /**