diff --git a/CHANGELOG.md b/CHANGELOG.md index bf83fc33a69..94db771058f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ All notable changes for each version of this project will be documented in this - `transitionToOrigin()` and `transitionTo()` now emit `transitioned` right away when the element is already at the target or there is no ghost to animate (e.g. its creation was canceled in `ghostCreate`). Previously no `transitioned` was emitted and the next drag did not start correctly. - `IgxExpansionPanelComponent` - `Alt + Arrow Down` and `Alt + Arrow Up` no longer expand or collapse the panel when its header is disabled. + - When the collapse animation ends, the body is no longer shown at full height for a frame before it is removed, e.g. in zoneless applications. - `IgxFilterDirective`, `IgxFilterPipe` - When filtering by multiple keys, an item matched by any of the keys is no longer hidden when the last key does not match, and `metConditionFn` / `overdueConditionFn` are called once per item instead of once per key. - Items whose `key` property has a falsy value, such as `0` or `false`, are now matched by that value instead of the item's text content. @@ -157,6 +158,7 @@ All notable changes for each version of this project will be documented in this - `next`, `prev` and focusing a step no longer throw when there is no active step, e.g. when all steps are initially disabled. In that case `activeStepChanging` is emitted with `oldIndex` set to `-1`. - `reset` keeps the active step visited, so its invalid state is displayed after moving to another step. - Setting `animationDuration` from code now applies it to the vertical animations as well. + - When the animation of the previously active step ends, its content is no longer shown for a frame before it is removed, e.g. in zoneless applications. Previously it was shown at full height in vertical orientation, and in its original place behind the active step's content in horizontal orientation. - `IgxTabsComponent` - With `manual` activation, a tab header focused with the keyboard is scrolled into view. - The selected tab header is scrolled into view when hidden tabs become visible, for example in a dialog or a drop-down. @@ -172,6 +174,8 @@ All notable changes for each version of this project will be documented in this - The dialog header no longer throws a missing locale data error for a `locale` without registered Angular locale data when `provideIgniteIntl()` is used. - The `aria-valuemin` and `aria-valuemax` of the selected seconds item no longer depend on the milliseconds of the value. - The default browser action of `Alt + Arrow Down` and `Alt + Arrow Up`, which open and close the drop-down, is now prevented. +- `IgxTreeComponent` + - When the collapse animation of a node ends, its child nodes are no longer shown at full height for a frame before they are removed, e.g. in zoneless applications. ## 22.2.6 diff --git a/projects/igniteui-angular/carousel/src/carousel/carousel-base.ts b/projects/igniteui-angular/carousel/src/carousel/carousel-base.ts index e6538fca1ba..af94d08629f 100644 --- a/projects/igniteui-angular/carousel/src/carousel/carousel-base.ts +++ b/projects/igniteui-angular/carousel/src/carousel/carousel-base.ts @@ -49,7 +49,10 @@ export abstract class IgxCarouselComponentBase implements OnDestroy { /** @hidden */ protected vertical = false; + private _animationOwnerDestroyed = false; + public ngOnDestroy(): void { + this._animationOwnerDestroyed = true; if (this.enterAnimationPlayer) { this.enterAnimationPlayer.destroy(); this.enterAnimationPlayer = null!; @@ -177,14 +180,19 @@ export abstract class IgxCarouselComponentBase implements OnDestroy { this.leaveAnimationPlayer = this.animationService.buildAnimation(animation, this.getPreviousElement()); this.leaveAnimationPlayer.animationEnd.subscribe(() => { + this.animationPosition = 0; + this.newDuration = 0; + this.leaveAnimationDone.emit(); + if (!this._animationOwnerDestroyed) { + // Render the left item's state synchronously, so that e.g. its content is removed before destroying + // the player clears the animation styles; otherwise it is painted in place for a frame. + this.cdr.detectChanges(); + } // TODO: animation may never end. Find better way to clean up the player if (this.leaveAnimationPlayer) { this.leaveAnimationPlayer.destroy(); this.leaveAnimationPlayer = null!; } - this.animationPosition = 0; - this.newDuration = 0; - this.leaveAnimationDone.emit(); }); this.leaveAnimationPlayer.play(); } diff --git a/projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.spec.ts b/projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.spec.ts index 13f21c888fe..e5521d1ce29 100644 --- a/projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.spec.ts +++ b/projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.spec.ts @@ -1154,6 +1154,40 @@ describe('igxExpansionPanel', () => { expect(headerButton.getAttribute('aria-expanded')).toEqual('false'); expect(headerElement.classList).not.toContain(CSS_CLASS_HEADER_EXPANDED); }); + + it('Should remove the body before the close animation player is reset', async () => { + const fixture = TestBed.createComponent(IgxExpansionPanelOnPushComponent); + fixture.detectChanges(); + const panel = fixture.componentInstance.panel; + const panelElement: HTMLElement = panel.nativeElement; + + panel.expand(); + await fixture.whenStable(); + fixture.detectChanges(); + expect(panelElement.querySelector('igx-expansion-panel-body')).not.toBeNull(); + + // capture whether the body is still rendered at the moment the close player resets the + // element styles; if it is, the body is painted at full height for a frame + let bodyRenderedOnReset: boolean | undefined; + const animationService = (panel as any).animationService; + const buildAnimation = animationService.buildAnimation.bind(animationService); + vi.spyOn(animationService, 'buildAnimation').mockImplementation((...args: any[]) => { + const player = buildAnimation(...args); + const reset = player.reset.bind(player); + vi.spyOn(player, 'reset').mockImplementation(() => { + bodyRenderedOnReset = !!panelElement.querySelector('igx-expansion-panel-body'); + reset(); + }); + return player; + }); + + panel.collapse(); + await fixture.whenStable(); + + expect(panel.collapsed).toBeTruthy(); + expect(bodyRenderedOnReset).toBe(false); + expect(panelElement.querySelector('igx-expansion-panel-body')).toBeNull(); + }); }); describe('Rendering tests: ', () => { diff --git a/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.spec.ts b/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.spec.ts index c66053dfd56..18e2e242e3a 100644 --- a/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.spec.ts +++ b/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.spec.ts @@ -1,11 +1,11 @@ import { TestBed } from '@angular/core/testing'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; -import { noop } from 'rxjs'; +import { noop, Subject } from 'rxjs'; import { IgxAngularAnimationService } from 'igniteui-angular/core'; import { ANIMATION_TYPE, ToggleAnimationPlayer } from './toggle-animation-component'; import { growVerIn, growVerOut } from 'igniteui-angular/animations'; import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { ElementRef } from '@angular/core'; +import { ChangeDetectorRef, ElementRef } from '@angular/core'; class MockTogglePlayer extends ToggleAnimationPlayer { } @@ -86,3 +86,66 @@ describe('Toggle animation component', () => { }); }); }); + +describe('Toggle animation component change detection', () => { + let animationEnd: Subject; + let mockPlayer: any; + let mockCdr: { detectChanges: ReturnType }; + + beforeEach(() => { + animationEnd = new Subject(); + mockPlayer = { + animationEnd, + position: 0, + init: vi.fn(), + play: vi.fn(), + hasStarted: vi.fn(() => false), + reset: vi.fn(), + destroy: vi.fn() + }; + mockCdr = { detectChanges: vi.fn() }; + TestBed.configureTestingModule({ + providers: [ + { provide: IgxAngularAnimationService, useValue: { buildAnimation: () => mockPlayer } }, + { provide: ChangeDetectorRef, useValue: mockCdr }, + MockTogglePlayer + ] + }); + }); + + it('Should render the owner after the close callback and before the close player is reset', () => { + const player = TestBed.inject(MockTogglePlayer); + const onDone = vi.fn(); + const doneSpy = vi.spyOn(player.closeAnimationDone, 'emit'); + + player.playCloseAnimation({ nativeElement: {} } as ElementRef, onDone); + expect(mockCdr.detectChanges).not.toHaveBeenCalled(); + animationEnd.next(); + + expect(mockCdr.detectChanges).toHaveBeenCalledTimes(1); + const detectChangesOrder = mockCdr.detectChanges.mock.invocationCallOrder[0]; + expect(onDone.mock.invocationCallOrder[0]).toBeLessThan(detectChangesOrder); + expect(doneSpy.mock.invocationCallOrder[0]).toBeLessThan(detectChangesOrder); + expect(detectChangesOrder).toBeLessThan(mockPlayer.reset.mock.invocationCallOrder[0]); + }); + + it('Should not render the owner when the open animation is done', () => { + const player = TestBed.inject(MockTogglePlayer); + + player.playOpenAnimation({ nativeElement: {} } as ElementRef); + animationEnd.next(); + + expect(mockPlayer.reset).toHaveBeenCalled(); + expect(mockCdr.detectChanges).not.toHaveBeenCalled(); + }); + + it('Should not render the owner when the close animation is done after it is destroyed', () => { + const player = TestBed.inject(MockTogglePlayer); + + player.playCloseAnimation({ nativeElement: {} } as ElementRef); + player.ngOnDestroy(); + animationEnd.next(); + + expect(mockCdr.detectChanges).not.toHaveBeenCalled(); + }); +}); diff --git a/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.ts b/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.ts index 42799d29269..9603e91c1fe 100644 --- a/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.ts +++ b/projects/igniteui-angular/expansion-panel/src/expansion-panel/toggle-animation-component.ts @@ -1,5 +1,5 @@ import { AnimationReferenceMetadata } from '@angular/animations'; -import { Directive, ElementRef, EventEmitter, inject, OnDestroy } from '@angular/core'; +import { ChangeDetectorRef, Directive, ElementRef, EventEmitter, inject, OnDestroy } from '@angular/core'; import { noop, Subject } from 'rxjs'; import { takeUntil } from 'rxjs/operators'; @@ -65,6 +65,13 @@ export abstract class ToggleAnimationPlayer implements ToggleAnimationOwner, OnD closeAnimation: growVerOut }; + /** + * Renders the owner synchronously when its close animation is done. Optional, as the player can be + * created outside of a view, e.g. as a provider. + */ + private readonly _toggleAnimationCdr = inject(ChangeDetectorRef, { optional: true }); + private _toggleAnimationDestroyed = false; + private closeInterrupted = false; private openInterrupted = false; @@ -85,6 +92,7 @@ export abstract class ToggleAnimationPlayer implements ToggleAnimationOwner, OnD /** @hidden @internal */ public ngOnDestroy() { + this._toggleAnimationDestroyed = true; this.destroy$.next(); this.destroy$.complete(); } @@ -151,6 +159,11 @@ export abstract class ToggleAnimationPlayer implements ToggleAnimationOwner, OnD if (!(type === ANIMATION_TYPE.OPEN ? this.openInterrupted : this.closeInterrupted)) { targetEmitter.emit(); } + if (type === ANIMATION_TYPE.CLOSE && !this._toggleAnimationDestroyed) { + // Render the collapsed state synchronously, so that the content is removed before resetting the player + // clears the animation styles; otherwise (e.g. zoneless) it is painted at full height for a frame. + this._toggleAnimationCdr?.detectChanges(); + } this.cleanUpPlayer(type); } diff --git a/projects/igniteui-angular/stepper/src/stepper/stepper.component.spec.ts b/projects/igniteui-angular/stepper/src/stepper/stepper.component.spec.ts index f9116b60b9f..cf0c73f7c56 100644 --- a/projects/igniteui-angular/stepper/src/stepper/stepper.component.spec.ts +++ b/projects/igniteui-angular/stepper/src/stepper/stepper.component.spec.ts @@ -1143,6 +1143,78 @@ describe('Rendering Tests', () => { expect((stepper as any).stepperService.visitedSteps).toContain(stepper.steps[firstAccessibleStepIdx]); }); + it('should remove the step content before the vertical collapse animation player is reset', async () => { + stepper.orientation = IgxStepperOrientation.Vertical; + stepper.steps[0].active = true; + fix.detectChanges(); + await fix.whenStable(); + const previousActiveStep = stepper.steps[0]; + const getContent = () => previousActiveStep.nativeElement.querySelector('[igxStepContent]'); + expect(getContent()).not.toBeNull(); + + // capture whether the content is still rendered at the moment the collapse player resets the + // container styles; if it is, the content is painted at full height for a frame + let contentRenderedOnReset: boolean | undefined; + const animationService = (previousActiveStep as any).animationService; + const buildAnimation = animationService.buildAnimation.bind(animationService); + vi.spyOn(animationService, 'buildAnimation').mockImplementation((...args: any[]) => { + const player = buildAnimation(...args); + if (args[1] === previousActiveStep.contentContainer.nativeElement) { + const reset = player.reset.bind(player); + vi.spyOn(player, 'reset').mockImplementation(() => { + contentRenderedOnReset = !!getContent(); + reset(); + }); + } + return player; + }); + const activeChangeSpy = vi.spyOn(previousActiveStep.activeChange, 'emit'); + + stepper.next(); + fix.detectChanges(); + await vi.waitFor(() => expect(activeChangeSpy).toHaveBeenCalledWith(false)); + + expect(contentRenderedOnReset).toBe(false); + expect(getContent()).toBeNull(); + }); + + it('should remove the step content before the horizontal leave animation player is destroyed', async () => { + stepper.orientation = IgxStepperOrientation.Horizontal; + stepper.steps[0].active = true; + fix.detectChanges(); + await fix.whenStable(); + const previousActiveStep = stepper.steps[0]; + // in horizontal orientation the content is rendered in the stepper body, not in the step element + const contentContainer: HTMLElement = previousActiveStep.contentContainer.nativeElement; + const getContent = () => contentContainer.querySelector('[igxStepContent]'); + expect(getContent()).not.toBeNull(); + + // capture whether the content is still rendered at the moment the leave player is destroyed, which + // clears its styles; if it is, the content is painted in place behind the active step for a frame + let contentRenderedOnDestroy: boolean | undefined; + const animationService = (stepper as any).animationService; + const buildAnimation = animationService.buildAnimation.bind(animationService); + vi.spyOn(animationService, 'buildAnimation').mockImplementation((...args: any[]) => { + const player = buildAnimation(...args); + if (args[1] === contentContainer) { + const destroy = player.destroy.bind(player); + vi.spyOn(player, 'destroy').mockImplementation(() => { + contentRenderedOnDestroy ??= !!getContent(); + destroy(); + }); + } + return player; + }); + const activeChangeSpy = vi.spyOn(previousActiveStep.activeChange, 'emit'); + + stepper.next(); + fix.detectChanges(); + await vi.waitFor(() => expect(activeChangeSpy).toHaveBeenCalledWith(false)); + + expect(contentRenderedOnDestroy).toBe(false); + expect(getContent()).toBeNull(); + }); + it('should properly collapse the previously active step in horizontal orientation and animation type \'fade\'', async () => { stepper.orientation = IgxStepperOrientation.Horizontal; stepper.horizontalAnimationType = 'fade'; diff --git a/projects/igniteui-angular/tree/src/tree/tree.spec.ts b/projects/igniteui-angular/tree/src/tree/tree.spec.ts index 7cedec4aa2e..a4909faf884 100644 --- a/projects/igniteui-angular/tree/src/tree/tree.spec.ts +++ b/projects/igniteui-angular/tree/src/tree/tree.spec.ts @@ -647,6 +647,35 @@ describe('IgxTree #treeView', () => { expect(['1', '2', '3']).toContain(getComponentSize(rootNode.nativeElement).trim()); }); + it('Should remove the node children before the collapse animation player is reset', async () => { + const node = tree.rootNodes[0]; + node.expanded = true; + fix.detectChanges(); + const getChildrenGroup = () => node.nativeElement.querySelector(':scope > .igx-tree-node__group'); + expect(getChildrenGroup()).not.toBeNull(); + + // capture whether the children are still rendered at the moment the collapse player resets the + // element styles; if they are, they are painted at full height for a frame + let childrenRenderedOnReset: boolean | undefined; + const animationService = (node as any).animationService; + const buildAnimation = animationService.buildAnimation.bind(animationService); + vi.spyOn(animationService, 'buildAnimation').mockImplementation((...args: any[]) => { + const player = buildAnimation(...args); + const reset = player.reset.bind(player); + vi.spyOn(player, 'reset').mockImplementation(() => { + childrenRenderedOnReset = !!getChildrenGroup(); + reset(); + }); + return player; + }); + + node.collapse(); + await vi.waitFor(() => expect(node.expanded).toBe(false)); + + expect(childrenRenderedOnReset).toBe(false); + expect(getChildrenGroup()).toBeNull(); + }); + it('Should do nothing when calling expand()/collapse() on expanded/collapsed node', () => { const expandingSpy = vi.spyOn(tree.nodeExpanding, 'emit'); const collapsingSpy = vi.spyOn(tree.nodeCollapsing, 'emit');