Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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

Expand Down
14 changes: 11 additions & 3 deletions projects/igniteui-angular/carousel/src/carousel/carousel-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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!;
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also redudant. From my understanding this is a guard to skip a change detection cycle if an animation is already destroyed but something else triggers it. Angular has alread marked the view destroyed and thus, cdr.detectChanges() is no-op.
Just leave a comment why this detectChanges is needed.

// 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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
}
Expand Down Expand Up @@ -86,3 +86,66 @@ describe('Toggle animation component', () => {
});
});
});

describe('Toggle animation component change detection', () => {
let animationEnd: Subject<void>;
let mockPlayer: any;
let mockCdr: { detectChanges: ReturnType<typeof vi.fn> };

beforeEach(() => {
animationEnd = new Subject<void>();
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();
});
});
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;

Expand All @@ -85,6 +92,7 @@ export abstract class ToggleAnimationPlayer implements ToggleAnimationOwner, OnD

/** @hidden @internal */
public ngOnDestroy() {
this._toggleAnimationDestroyed = true;
this.destroy$.next();
this.destroy$.complete();
}
Expand Down Expand Up @@ -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) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

animationEnd subscription has takeUntil so this might never be invoked with toggleAnimationDestroyed set to true. I dont think you need that flag here.

// 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
29 changes: 29 additions & 0 deletions projects/igniteui-angular/tree/src/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading