angular / components

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

[Tabs] Toggling quickly can cause content to be duplicated #10938

Open benelliott opened 6 years ago

benelliott commented 6 years ago

Bug, feature request, or proposal:

I have had some trouble reproducing this one reliably (it seems to occur more often on my app than in reproductions) but there seems to be a race condition related to the rendering of tab content when it is lazy-loaded.

What is the expected behavior?

Tab content should only be displayed once.

What is the current behavior?

Toggling tabs very quickly (using arrow keys + space bar) can cause tab content to be drawn multiple times.

What are the steps to reproduce?

StackBlitz: https://stackblitz.com/edit/angular-material2-issue-h6pgut?file=app%2Fapp.component.ts

I am intentionally trying to be awkward in terms of timing (ie. setting the selectedTabIndex in a timeout), but this behaviour still should not happen.

I have recorded a reproduction in this video: https://gfycat.com/ReadyFairBasil Note that at the start there is only one instance of each tab's content but by the end the second tab's content is duplicated.

What is the use-case or motivation for changing an existing behavior?

Bug

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Latest StackBlitz (Angular 6-rc2, Material 6-rc1)

Is there anything else we should know?

I could only reproduce by using lazy-loaded tabs + NgForTrackBy + OnPush + window.setTimeout, but these may not be necessary. I also had to use keyboard selection to toggle fast enough to trigger the issue.

benelliott commented 6 years ago

@andrewseguin Here is a much clearer repro: https://stackblitz.com/edit/angular-material2-issue-p51bcx

I tried to look into it myself and it seems the issue is quite deep into the Portal/animation/ViewContainerRef logic.

The older Tab 1 content ViewRef is marked as destroyed but still in the DOM.

xyz247 commented 5 years ago

It looks like anything less than 500ms will reproduce this bug? anything can be done using a delay trigger?

manklu commented 5 years ago

@benelliott The problem is the animation. If you disable it, the problem disappears.

<mat-tab-group [@.disabled]="true">
  <mat-tab *ngFor="let tab of tabs" [label]="tab">
    <div *matTabContent class="content">
      {{tab}}
    </div>
  </mat-tab>
</mat-tab-group>

This would also fit the ~500ms observation from @xyz247.

Possibly related to https://github.com/angular/angular/issues/26133

simeyla commented 5 years ago

@andrewseguin I'm seeing this in my real app. How come issues like this keep going unfixed after months and months? There are several duplicate reports of this one - so it's affecting real people.

It's easily duplicated with the lazy load demo for tabs.

It's clearly some race condition with the detachment / attachment of portals - it shouldn't be too hard to fix. (and I only see this only on lazy load)

joelcoxokc commented 4 years ago

Simple Workaround

This only works if you have only one base element inside each tabContent template

For Example

<mat-tab-group>
    <mat-tab>
        <ng-template matTabContent>
            <div class="tab-container">
                <!-- Content goes here -->
            </div>
        </ng-template>
    </mat-tab>
    <mat-tab>
        <ng-template matTabContent>
            <div class="tab-container">
                <!-- Content goes here -->
            </div>
        </ng-template>
    </mat-tab>
</mat-tab-group>

Listen to the output on the <mat-tab-group> for (animationDone)

<mat-tab-group (animationDone)="onTabAnimationDone()">

On Animation done, we want to look for any additional .tab-containers that have been rendered inside the active tab.

export class MyComponent {
    constructor(private elementRef: ElementRef) {}

    public onTabAnimationDone(): void {
        const inactiveTabs = this.elementRef.nativeElement.querySelectorAll(
            '.mat-tab-body-active .mat-tab-body-content > .tab-container:not(:first-child)'
        );

        inactiveTabs.forEach(tab => tab.remove());
    }
}
francisaguilar21 commented 4 years ago

Is there any update on this issue? I'm still experiencing this even now

JoeyLi-1 commented 3 years ago

I had experienced same issue. My solution is: Do not use lazy load tab content. Hope this could help someone.

BenLune commented 2 years ago

Hi, I'm surprised a so obvious bug is not solved since 2018. I will deal with it.

BenLune commented 2 years ago

I think the lazyloading part of the Tabs Component is not really ready, maybe you should not communicate about it. It seems also to get problems with ChangeDetection. On one tab I had to click on the tab, and then outside to trigger changeDetection and get my tab selected... Removing tab content lazyloading solved all the problems.

onra2 commented 2 years ago

any update to this? it's been years

joaoa-casagrande commented 1 year ago

Simple Workaround

This only works if you have only one base element inside each tabContent template

For Example

<mat-tab-group>
    <mat-tab>
        <ng-template matTabContent>
            <div class="tab-container">
                <!-- Content goes here -->
            </div>
        </ng-template>
    </mat-tab>
    <mat-tab>
        <ng-template matTabContent>
            <div class="tab-container">
                <!-- Content goes here -->
            </div>
        </ng-template>
    </mat-tab>
</mat-tab-group>

Listen to the output on the <mat-tab-group> for (animationDone)

<mat-tab-group (animationDone)="onTabAnimationDone()">

On Animation done, we want to look for any additional .tab-containers that have been rendered inside the active tab.

export class MyComponent {
    constructor(private elementRef: ElementRef) {}

    public onTabAnimationDone(): void {
        const inactiveTabs = this.elementRef.nativeElement.querySelectorAll(
            '.mat-tab-body-active .mat-tab-body-content > .tab-container:not(:first-child)'
        );

        inactiveTabs.forEach(tab => tab.remove());
    }
}

Another Workaround

Disable tabs while it's changing:

For Example

<mat-tab-group (animationDone)="onAnimationDone()" (selectedIndexChange)="indexChange()" [selectedIndex]="0">

    <mat-tab [disabled]="animationRunning">
        <ng-template matTabContent>
             <!-- Content goes here -->
        </ng-template>
    </mat-tab>
    <mat-tab [disabled]="animationRunning" >
        <ng-template matTabContent>
             <!-- Content goes here -->
        </ng-template>
    </mat-tab>
    <mat-tab [disabled]="animationRunning">
        <ng-template matTabContent>
             <!-- Content goes here -->
        </ng-template>
    </mat-tab>
</mat-tab-group>
export class MyComponent {
    private _animationRunning = false;

    get animationRunning(){ return  this._animationRunning};

    indexChange(){
        this._animationRunning = true;
    }

    onAnimationDone()
    {
        this._animationRunning = false;
    }
}
JonasDev17 commented 9 months ago

@andrewseguin Do you have any updates on this? Will the bug not be fixed?

zhmeka commented 1 month ago

any updates?