Material: panel: mdPanelRef.show() does not support re-opening a dialog w/ an animation

[saurabh-shelgaonkar]

Bug, enhancement request, or proposal:

Bug

CodePen and steps to reproduce the issue:

https://codepen.io/saurabh-shelgaonkar/pen/LmqRZz?editors=0010

Detailed Reproduction Steps:

1. This is a codepen from AngularJs Material Website for Panel animation
2. Click on Dialog to Open the Dialog, then close the dialog with close button. Here I have used mdPanelRef.hide() to hide the panel.
3. Open the panel with Show button. It will not appear.
4. Comment the animation in config. Repeat the above steps, the panel appears.

What is the expected behavior?

The panel should show and hide with animations in config.

What is the current behavior?

The panel doesn't show and hide with animations in config.

What is the use-case or motivation for changing an existing behavior?

It is important to maintain the panel state many a times, so using show and hide.

Which versions of AngularJS, Material, OS, and browsers are affected?

• AngularJS: 1.6.7
• AngularJS Material: 1.1.9
• OS:
• Browsers: Chrome

Is there anything else we should know? Stack Traces, Screenshots, etc.

No

[codymikol]

I got your scenario to work in this pen https://codepen.io/codymikol/pen/XqGrPB?editors=0011

[saurabh-shelgaonkar]

Hey,
Thanks for the reply.

I went through the codepen.
If I am not mistaken, you have opened a new dialog/panel on Show button click.

In my scenario, I want to maintain the state of previously opened dialog, by hiding it, and showing it again via mdPanelRef.

Please verify.

[codymikol]

Hey, you're absolutely right, I set the whole thing up so I could open the panel reference and then went ahead and made a new panel (Not enough coffee yesterday). I updated my pen so it actually calls the reference panel. I am having the same errors as you.

https://codepen.io/codymikol/pen/XqGrPB?editors=1111

It looks like the return animation on the panel gets mangled somehow. If you try to reproduce this selecting the slide animation type the panel does return, but way off to the left.

EDIT: It looks like the animation is going in to where the previous out animation was. You can see that I'm working on making the pen as simple as possible now to help narrow down what the issue is.

[NgMomentum]

This might be related (or not), but I add this code:

        // Make sure it is an array
    if (angular.isString(self.config.groupName)) {
            self.config.groupName = [self.config.groupName];
        }

...to the MdPanelRef.prototype.open function in "material.component.panel" to catch input for config.groupName which is a string, and not an array. It appears that using the MdPanelRef.prototype.open function misses the similar validation used in MdPanelService.prototype.create which catches and forces config.groupName to be an array for later processing.

[Splaktar]

@NgMomentum I gave that a shot, but it did not help with fixing this issue unfortunately.

[Splaktar]

OK, I've investigated this a bit further. The issue appears to be that MdPanelAnimation.prototype.animateOpen and MdPanelAnimation.prototype.animateClose do not properly support reopening of panels.

On open, _md-panel-animate-enter gets applied along with style="z-index: 151; top: 0px; right: 0px;".
On close, _md-panel-animate-scale-out _md-panel-animate-leave gets applied, but the _md-panel-animate-enter is not removed.

So on reopen, no animation happens because the _md-panel-animate-enter class is already there. Additionally, the transform CSS calculations fail on the second open causing the dialog to open in a backwards way with an end state of it taking up the location of the 'Dialog' button with style="z-index: 151; top: 0px; right: 0px; transform: translate3d(-1298px, 248px, 0px) scale(0.18, 0.08);". The transform should have been removed when it was reopened.

[Splaktar]

I fixed the issue with classes getting left around after the animations are rerun in https://github.com/angular/material/commit/9c63a0576ba34aae68e30cd76ffc48e77bb3b870.

However, I was not able to solve the issue with the CSS transforms. It requires digging deeply into https://github.com/angular/material/blob/fixPanelReopenAnimation/src/core/util/animation/animate.js, which I have never looked at before. Additionally, the md-panel code is not well documented from a developer standpoint, so I'm not sure what the intention of a number of these functions is.

I'm going to move this to the backlog as I don't anticipate that this will be solved anytime soon. If someone from the community wants to help debug and investigate (and makes some progress), I can take another look.

[oliversalzburg]

I investigated this and it appears that removing the animation classes was really never implemented. So fully fixing this would probably require fully cleaning up all related classes when the panel is closed.

The easiest solution I could think of, would be to remove the "open" animation classes when an animation wants to perform the "close" animation here:

https://github.com/angular/material/blob/master/src/components/panel/panel.js#L3409-L3465

For the time being, this works as a workaround: You can manually remove the "close" classes in the onRemoving hook of the panel. Hooking into the open process is not necessary.

onRemoving : panelReference => {
    panelReference.panelEl[ 0 ].classList.remove( "_md-panel-animate-scale-out", "_md-panel-animate-leave" );
    return null;
}

I'm not sure if any internal components make use of the feature of compiling a panel once and then showing/hiding it, but if any component does that, I could imagine that some other defects also trace back to this.

[oliversalzburg]

What confused me was that $$mdAnimate.translate3d will return a new animation explicitly designed to reverse the previous one. If my interpretation of it is correct, then this is also designed to remove the classes which are not cleaned up here.

This reversal animation is ultimately ignored in the promise chain that handles open/close of a panel.

I'm not sure if not using this reversal animation is an oversight or if manual cleanup was required all along. However, the animateClose and animateOpen functions explicitly provide reverseAnimationOptions. This indicates to me that MdPanelAnimation to some degree expected this reversal to be used.

Would love to get some thoughts on this.

[Splaktar]

That's interesting. Are you able to get the reversal animation to not be ignored? That seems like the cleanest solution.

Can you point to the code where this ignoring happens?

[oliversalzburg]

We call
https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L1634

which creates the animatePromise that is resolved when self._animateOpen is resolved

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L1646

self._animateOpen resolves when animationConfig.animateOpen resolves

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L2299

MdPanelAnimation.animateOpen resolves when animator.translate3d resolves (this is the promise that resolves to the animation reversal method)

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L3341

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/core/util/animation/animate.js#L33

If we now take a couple steps back to the beginning, the reversal function ends up in the animatePromise, but it is unused:

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L1664

I've used _open_ as an example here, but the same behavior is true for _close_.

[Splaktar]

So then in
https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L2299-L2300

We should be doing something like the following

    animationConfig.animateClose(self.panelEl)
        .then(function(reverseTranslate) {
           // TODO what to do with reverseTranslate? Store it until close is called?
           self._done(resolve, self);
        }, warnAndClose);

But looking at
https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/core/util/animation/animate.js#L37-L46

And then animateOpen and animateClose, they appear to be specifically designed to ignore the existence of reverseTranslate.

While this would continue to ignore reverseTranslate, would just adding valid transitionOutClasses to the different cases in animateOpen and animateClose solve this?

[Splaktar]

It seems like animateOpen and animateClose apply the transitionInClass and transitionOutClass properly when custom open and close animation classes are supplied as documented here.

https://github.com/angular/material/blob/0b83a2cbfd527a171fef2d0ffef038ff8a8dd674/src/components/panel/panel.js#L3389-L3392

That may be a path to quickly verifying if this approach works or not?

[Splaktar]

Yep, that looks like it works. This CodePen changes JS line 41 to

.withAnimation({open: '_md-panel-animate-enter', close: '_md-panel-animate-leave'});

Which causes opening to work repeatedly.

@oliversalzburg are you interested in sending a PR for this?

[oliversalzburg]

@oliversalzburg are you interested in sending a PR for this?

Very much so! Thanks for the input. I will give this another close look.

[oliversalzburg]

@Splaktar I submitted #11856, which should fix this issue.