Skip to content

fix(expansion-panel): emitting events twice on some browsers - #13600

Merged
josephperrott merged 1 commit into
angular:masterfrom
crisbeto:expansion-duplicate-events
Oct 19, 2018
Merged

fix(expansion-panel): emitting events twice on some browsers#13600
josephperrott merged 1 commit into
angular:masterfrom
crisbeto:expansion-duplicate-events

Conversation

@crisbeto

Copy link
Copy Markdown
Member

Fixes the expansion panel emitting the afterExpand and afterCollapse events twice on IE and Edge due to a bug in @angular/animations.

Fixes the expansion panel emitting the `afterExpand` and `afterCollapse` events twice on IE and Edge due to a bug in `@angular/animations`.
@crisbetocrisbeto added the target: patch This PR is targeted for the next patch release label Oct 13, 2018
@googlebotgooglebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 13, 2018
crisbeto added a commit to crisbeto/material2 that referenced this pull request Oct 14, 2018
Along the same lines as angular#13600 and angular#13587. The `animationDone` will be emitted twice in a row for some browsers due to a bug in `@angular/animations`.
this._bodyAnimationDone.pipe(distinctUntilChanged((x, y) => {
return x.fromState === y.fromState && x.toState === y.toState;
})).subscribe(event => {
if (event.fromState !== 'void') {

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.

Instead of removing the _bodyAnimation method, can we call it here since an upstream fix to this from angular/angular will likely have us essentially reverting this change?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I don't mind re-adding, but I don't see why it's an issue, considering that it's a private API. Once the upstream fix gets merged, we can turn it back into a private method.

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.

Yeah, its fine to leave it as is. I just wanted us to consider it.

@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

this._bodyAnimationDone.pipe(distinctUntilChanged((x, y) => {
return x.fromState === y.fromState && x.toState === y.toState;
})).subscribe(event => {
if (event.fromState !== 'void') {

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.

Yeah, its fine to leave it as is. I just wanted us to consider it.

@josephperrottjosephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Oct 15, 2018
josephperrott pushed a commit that referenced this pull request Oct 19, 2018
…#13608)
Along the same lines as #13600 and #13587. The `animationDone` will be emitted twice in a row for some browsers due to a bug in `@angular/animations`.
josephperrott pushed a commit that referenced this pull request Oct 19, 2018
…#13608)
Along the same lines as #13600 and #13587. The `animationDone` will be emitted twice in a row for some browsers due to a bug in `@angular/animations`.
josephperrott pushed a commit that referenced this pull request Oct 19, 2018
…#13608)
Along the same lines as #13600 and #13587. The `animationDone` will be emitted twice in a row for some browsers due to a bug in `@angular/animations`.
@josephperrott
josephperrott merged commit fa6e46d into angular:masterOct 19, 2018
@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 Sep 9, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: mergeThe PR is ready for merge by the caretakercla: yesPR author has agreed to Google's Contributor License Agreementtarget: patchThis PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@crisbeto@josephperrott@googlebot