Skip to content

fix(material/expansion): panel appearing as open when parent is animating away - #11778

Closed
crisbeto wants to merge 1 commit into
angular:mainfrom
crisbeto:11765/expansion-animation
Closed

fix(material/expansion): panel appearing as open when parent is animating away#11778
crisbeto wants to merge 1 commit into
angular:mainfrom
crisbeto:11765/expansion-animation

Conversation

@crisbeto

Copy link
Copy Markdown
Member

Fixes an issue where the expansion panel will appear as if it is open, if the element is part of a component that is animating away. The issue comes from the fact that Angular resets the animation state to void which we don't have in the animation definitions.

Fixes#11765.

@googlebotgooglebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 13, 2018
jelbourn
jelbourn previously approved these changes Jun 13, 2018

@jelbournjelbourn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jelbournjelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jun 13, 2018
josephperrott
josephperrott previously approved these changes Jun 18, 2018

@josephperrottjosephperrott left a comment

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.

LGTM

@crisbeto
crisbetoforce-pushed the 11765/expansion-animation branch from a019463 to 39ddff9CompareAugust 20, 2018 20:25
@ngbot

ngbotBot commented Sep 18, 2018

Copy link
Copy Markdown

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

2 similar comments
@ngbot

ngbotBot commented Sep 18, 2018

Copy link
Copy Markdown

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@ngbot

ngbotBot commented Sep 18, 2018

Copy link
Copy Markdown

Hi @crisbeto! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@Teamop

Teamop commented Nov 28, 2018

Copy link
Copy Markdown
Contributor

Hi @jelbourn@crisbeto , seems this PR hasn't been merged for a long time after being approved. For the checks, seems should rerun as a lot of them are just pending. Do you have other concerns to update?
seems related to #13870

@stevenmcountryman

Copy link
Copy Markdown

Hi @jelbourn@crisbeto !

We're also desperately waiting for this to be merged in. All of our expansion panels are affected by this bug and it's very jarring.

@simeyla

Copy link
Copy Markdown

@crisbeto Been spending all afternoon playing around and trying to understand this issue.

Adding a class of .ng-animating { outline: 1px solid red; } gives me the following screenshot when animating my panel onscreen for the first time. So all the little expander icons and the panels themselves get :ng-animating which they're inheriting somehow.

image

Even if my slide animation on the 'outside' has an explicit css class and { limit: 1 } applied it still permeates into the accordion. The only way I managed to block it was by adding [@.disabled] to the accordion.`

What I'm really having a difficult time understanding is why all the controls inside accordion are able to inherit .ng-animating from the outside in the first place. If that's just the way angular works then isn't a change needed in that behavior to be able to get anywhere with fixing things like this? (and more complicated than what you said about void state?)

This is most frustrating because you think animating a panel off screen will take about 15 minutes to do and then everything exploded in my face (literally!!)

@mmalerbammalerba added aaa and removed aaa labels Apr 25, 2019
@am-awais

Copy link
Copy Markdown

Please Solve this Issue ASAP.
Its really annoying!!!!!!

@andrewseguinandrewseguin added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label May 30, 2019
@crisbeto
crisbetoforce-pushed the 11765/expansion-animation branch from 0b2dde5 to 639b3c2CompareJanuary 1, 2020 16:44
@crisbeto

Copy link
Copy Markdown
MemberAuthor

Rebased and bumping to a P2, because the issue keeps getting reported.

@crisbetocrisbeto added P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jan 1, 2020
@andrewseguin

Copy link
Copy Markdown
Contributor

I think there is an unintended side-effect of this change. If your expansion panel starts as expanded, you see the animation occur on the page when loaded.

You can see this if you change the demo's expansion panel to be expanded, then load the page. The animation will render, when I think what we expect is that the expansion panel just initializes as open.

<mat-expansion-panel class="demo-expansion-width" #myPanel
expanded
(afterExpand)="addEvent('afterExpand')"
(afterCollapse)="addEvent('afterCollapse')">
...
</mat-expansion-panel>

@andrewseguinandrewseguin removed the action: merge The PR is ready for merge by the caretaker label Jul 7, 2020
@BazylPL

Copy link
Copy Markdown

any news on that conflict?

@mmalerbammalerba removed the lgtm label Jul 31, 2020
@crisbetocrisbeto changed the title fix(expansion): panel appearing as open when parent is animating awayfix(material/expansion): panel appearing as open when parent is animating awayMay 31, 2021
…ting away
Fixes an issue where the expansion panel will appear as if it is open, if the element is part of a component that is animating away. The issue comes from the fact that Angular resets the animation state to `void` which we don't have in the animation definitions.
Fixesangular#11765.
@crisbeto
crisbetoforce-pushed the 11765/expansion-animation branch from 639b3c2 to 16fe90eCompareMay 31, 2021 17:43
@crisbetocrisbeto added action: merge The PR is ready for merge by the caretaker and removed needs rebase labels May 31, 2021
@andrewseguinandrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 29, 2021
@andrewseguin
andrewseguin dismissed stale reviews from josephperrott and jelbourn via 16fe90eMay 2, 2022 20:33
@OlliHolli

Copy link
Copy Markdown

Seeing that this is still not merged I have found another workaround.
When declaring the animations simply add fitting animationstates.

animations: [
trigger( "detailExpand", [
state( "collapsed", style( { height: "0px", minHeight: "0" } ) ), state( "void", style( { height: "0px", minHeight: "0" } ) ), state( "expanded", style( { height: "*" } ) ), transition( "expanded <=> collapsed", animate( "225ms cubic-bezier(0.4, 0.0, 0.2, 1)" ) ), transition( "expanded <=> void", animate( "225ms cubic-bezier(0.4, 0.0, 0.2, 1)" ) ) ] ) ]

Notice that I added a "void" state and transition.

It is duplicate code, but other than that I did not encounter any issues with it.

@josephperrottjosephperrott added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed needs rebase labels Nov 16, 2022
@crisbeto

Copy link
Copy Markdown
MemberAuthor

Closing in favor of #30119.

@crisbetocrisbeto closed this Dec 5, 2024
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-botangular-automatic-lock-botBot locked and limited conversation to collaborators Jan 5, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanupThe PR is in need of cleanup, either due to needing a rebase or in response to comments from reviewsaction: mergeThe PR is ready for merge by the caretakerP2The issue is important to a large percentage of users, with a workaroundtarget: patchThis PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mat-expansion-panel "opens" with angular enter-leave animations, although current state is closed.

12 participants

@crisbeto@Teamop@stevenmcountryman@simeyla@am-awais@andrewseguin@BazylPL@OlliHolli@jelbourn@josephperrott@googlebot@mmalerba