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
| if (!(type === ANIMATION_TYPE.OPEN ? this.openInterrupted : this.closeInterrupted)) { | ||
| targetEmitter.emit(); | ||
| } | ||
| if (type === ANIMATION_TYPE.CLOSE && !this._toggleAnimationDestroyed) { |
There was a problem hiding this comment.
animationEnd subscription has takeUntil so this might never be invoked with toggleAnimationDestroyed set to true. I dont think you need that flag here.
| this.animationPosition = 0; | ||
| this.newDuration = 0; | ||
| this.leaveAnimationDone.emit(); | ||
| if (!this._animationOwnerDestroyed) { |
There was a problem hiding this comment.
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.
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)