angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.39k stars 6.76k forks source link

bug(cdk/scrolling): virtual-scroll flickers during scroll in zoneless mode #29174

Open OonihiloO opened 6 months ago

OonihiloO commented 6 months ago

Is this a regression?

The previous version in which this bug was not present was

Introduces in v18, as related to zoneless application feature.

Description

When provideExperimentalZonelessChangeDetection is enabled, virtual-scroll-viewport flickers during scroll because the transform operation becomes visible.

Reproduction

StackBlitz link: https://stackblitz.com/edit/stackblitz-starters-xwe1bt?file=src%2Fcdk-scroll-component.ts Steps to reproduce:

  1. Scroll slowly, as with a trackpad, not a mouse wheel
  2. Scrolled elements flickers during scroll: the transform operation is visible to the end user

Expected Behavior

See this StackBlitz link: https://stackblitz.com/edit/stackblitz-starters-xwe1bt?file=src%2Fcdk-scroll-component.ts It's the same code, but with zone enabled: Steps to reproduce:

  1. Scroll slowly, as with a trackpad, not a mouse wheel
  2. Scrolled elements don't flicker during scroll: the transform operation is invisible to the end user

Actual Behavior

StackBlitz link: https://stackblitz.com/edit/stackblitz-starters-xwe1bt?file=src%2Fcdk-scroll-component.ts Steps to reproduce:

  1. Scroll slowly, as with a trackpad, not a mouse-wheel
  2. Scrolled elements flickers during scroll: the transform operation is visible to the end user

Environment

Angular CLI: 18.0.2 Node: 18.20.3 Package Manager: npm 10.2.3 OS: linux x64

Angular: 18.0.1 ... animations, cdk, common, compiler, compiler-cli, core, forms ... material, platform-browser

Package Version

@angular-devkit/architect 0.1800.2 @angular-devkit/build-angular 18.0.2 @angular-devkit/core 18.0.2 @angular-devkit/schematics 18.0.2 @angular/cli 18.0.2 @schematics/angular 18.0.2 rxjs 7.8.1 typescript 5.4.5

Klaster1 commented 5 months ago

Any updated on this? Encountered the same behavior in my app, thought this was my fault, but even reducing to the most minimal markup didn't help. Running Angular 18 and zoneless.

OonihiloO commented 5 months ago

Just to confirm that the issue is sill present in 18.1.0 release. The reproductions have been updated with the latest versions of the Angular stack.

xyzmaker1 commented 3 months ago

This is also an issue in the latest release (18.2.1). Is there a known workaround for this issue?

OonihiloO commented 3 months ago

@xyzmaker1 none that I know... The code to fix this issue seems to have been stabilized in angular (the afterNextRender api), but it seems that zoneless being still experimental, the team has other priority.

xyzmaker1 commented 3 months ago

@xyzmaker1 none that I know... The code to fix this issue seems to have been stabilized in angular (the afterNextRender api), but it seems that zoneless being still experimental, the team has other priority.

@OonihiloO - Thanks for the reply. I saw the PR that you pushed to try to resolve this issue: https://github.com/angular/components/pull/29173.

I took this as a starting point to test this on my end. If I run the changes that your PR had, I can see that a number of tests fail , but if I only make this change:

  /** Run change detection. */
  private _doChangeDetection() {
    if (this._isDestroyed) {
      return;
    }

    this.ngZone.run(() => {
      // Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
      // from the root, since the repeated items are content projected in. Calling `detectChanges`
      // instead does not properly check the projected content.
      this._changeDetectorRef.markForCheck();
      afterNextRender(
        () => {
          // Apply the content transform. The transform can't be set via an Angular binding because
          // bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
          // string literals, a variable that can only be 'X' or 'Y', and user input that is run through
          // the `Number` function first to coerce it to a numeric value.
          this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;

          this._isChangeDetectionPending = false;
          const runAfterChangeDetection = this._runAfterChangeDetection;
          this._runAfterChangeDetection = [];
          for (const fn of runAfterChangeDetection) {
            fn();
          }
        },
        {injector: this._injector},
      );
    });
  }

The tests appear to pass and the scrolling issue appears to be resolved. This solution appears to be fine but I'll admit that this is the first time that I've ever dug into this codebase before so I'm sure there's something incorrect about this approach. This approach is slightly different than yours as I've kept the logic all within one afterNextRender block.