Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A regression test is missing for synchronous body removal before animation reset, and existing collapse tests also pass with the old behavior, leaving the flicker unguarded.
1 open issue
What changed in this PR
This PR addresses #17964 by rendering expansion-panel collapse synchronously to prevent flicker in zoneless apps, including accordion and banner components built on the panel.
Changes:
- Replaces
markForCheck()withdetectChanges()after collapse. - Adds a comment explaining why the body must be removed before the close-animation player resets.
| File | Description |
|---|---|
| projects/igniteui-angular/expansion-panel/src/expansion-panel/expansion-panel.component.ts | Makes collapse rendering synchronous to prevent a full-height flash after animation. |
Review effort: Balanced
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mddragnev
left a comment
mddragnev
left a comment
There was a problem hiding this comment.
This seems to be a more general issue. The tree and the vertical stepper have the same issue. Might be better to fix this in the AnimationPlayer instead.
The expansion panel, tree and stepper removed their collapsed content only on the next render, after the animation player was reset or destroyed. The content was then painted in place for a frame, e.g. in zoneless applications. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
teodosiah
changed the title
mddragnev
left a comment
mddragnev
left a comment
There was a problem hiding this comment.
@teodosiah I think you should backport this as well, right?
viktorkombov
added
💥 status: in-test
viktorkombov
left a comment
viktorkombov
left a comment
There was a problem hiding this comment.
The fix works for the expansion panel, accordion, banner, tree and stepper. Found two problems, details inline:
| if (type === ANIMATION_TYPE.CLOSE) { | ||
| // 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(); |
There was a problem hiding this comment.
A Next button inside a step's form no longer submits it, and a value with updateOn: 'submit' is lost (master is fine).
Setup: zoneless app (or zone.js with eventCoalescing); <igx-stepper orientation="vertical" verticalAnimationType="fade"> (or "none"); in the first step's igxStepContent: <form [formGroup]="form" (ngSubmit)="save()"><input formControlName="name"><button (click)="stepper.next()">Next</button></form>.
Steps: type a name → click Next → save() is not called and form.value.name stays empty. A document:click handler that checks el.contains(event.target) also sees this click as outside.
Why: without a close animation, onDoneHandler runs inside next(), so this detectChanges() removes the form and the clicked button while the click is still being dispatched. The same happens with provideNoopAnimations() for any orientation and animation type. Maybe render here only after a real close animation?
| this.leaveAnimationDone.emit(); | ||
| // 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(); |
There was a problem hiding this comment.
The carousel can throw from the leave animation end, and then the leave player is not destroyed (master is fine).
Setup: zoneless app; <igx-carousel> with @for (item of items(); track item.id) { <igx-slide>{{ item.title }}</igx-slide> } and <ng-template igxCarouselIndicator let-slide>{{ items()[slide.index].title }}</ng-template> (the carousel dev demo reads slides[slide.index] the same way).
Steps: carousel.next() → when the slide animation finishes (Promise.all(carouselElement.getAnimations({ subtree: true }).map(a => a.finished))), remove the slide that left from items() → TypeError: Cannot read properties of undefined (reading 'title').
Why: this detectChanges() renders the indicators with the old slides list before the app view is checked, and the error skips the destroy() below.
Closes #17964
Description
Motivation / Context
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)