Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MdDialogRef.afterClosed() emits before dialog refocuses on an underlying element #3890

Closed
benelliott opened this issue Apr 4, 2017 · 7 comments
Assignees

Comments

@benelliott
Copy link
Contributor

benelliott commented Apr 4, 2017

Bug, feature request, or proposal:

The MdDialogRef.afterClosed() observable emits an event before the MdDialogContainer has restored focus to an underlying element. This can cause problems if subscriptions to MdDialogRef.afterClosed() involve triggering focus events themselves.

This happens because MdDialogContainer waits for the NgZone's onMicrotaskEmpty to emit before executing its refocus operation.

What is the expected behavior?

If this is the intended behaviour then I believe it should be made explicit in the documentation. Otherwise, the MdDialogRef.afterClosed() observable should only emit once all dialog cleanup has been completed.

Currently, the documentation says:

afterClosed - Gets an observable that is notified when the dialog is finished closing.

What is the current behavior?

See above

What are the steps to reproduce?

See this plunker. AppComponent wants to focus its input element after the dialog has closed but this event is overwritten by MdDialogContainer, which restores focus to the button element.

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

The contract with MdDialogRef.afterClosed() should be clear.

I am using MdDialog within the context of an application that refocuses elements in a non-trivial way so it would not be possible to control this behaviour through tabindex attributes.

Which versions of Angular, Material, OS, browsers are affected?

Tested on Angular 4.0.1/Material head f40296e/Windows 7/Chrome 57

Is there anything else we should know?

The workaround I am using is to perform the focus event inside another NgZone.onMicrotaskEmpty subscription:

this.modal.afterClosed().subscribe(() => {
    this.ngZone.onMicrotaskEmpty.first().subscribe(() => {
        // Focus operation here
    });
});
@benelliott benelliott changed the title MdDialogRef.afterClosed() emits before dialog is fully closed MdDialogRef.afterClosed() emits before dialog refocuses on an underlying element Apr 4, 2017
@crisbeto crisbeto self-assigned this Apr 4, 2017
@crisbeto crisbeto added the has pr label Apr 4, 2017
@ghost
Copy link

ghost commented Apr 25, 2017

I have a related(?) problem when I have two dialogs open (bad UX maybe but it's not my design), and I try to close one as soon as the other is closed.

onClickRemove() {
  this.dialog.open(MyConfirmComponent, {
    data: {
      message: 'Really remove this item?',
      confirmActionLabel: 'Remove'
    }
  }).afterClosed().subscribe((result?: boolean) => {
    if (result) { // user clicked ok
      this.dialogRef.close(); // close original dialog
    }
  });
}

When this runs, the page will get stuck in a weird state where the first .cdk-global-overlay-wrapper element is still in the DOM. The dialog is invisible, but the underlying page ignores pointer events and doesn't scroll.

The onMicrotaskEmpty thing worked for me. Thanks!

@crisbeto
Copy link
Member

Sounds weird @dbandstra, I'll take a look. Also, if you want to close all your open dialogs, you can use MdDialog.closeAll.

@crisbeto
Copy link
Member

crisbeto commented Apr 29, 2017

I wasn't able to reproduce your issue @dbandstra. Here's what I tried:

openDialogs() {
  this.dialogRef = this.dialog.open(JazzDialog, {
    data: { message: 'dialog 1' }
  });

  this.otherDialogRef = this.dialog.open(JazzDialog, {
    data: { message: 'dialog 2' }
  });

  this.otherDialogRef.afterClosed().subscribe(() => {
    this.dialogRef.close();
  });
}

Can you post a Plunkr that shows it? Also I'll close this issue since it should've been fixed by #3875.

@ghost
Copy link

ghost commented May 4, 2017

Hi @crisbeto, I think the visibly glitchy behaviour was a result of some styles I had overridden.

Anyway, I made a Plunkr: http://plnkr.co/edit/0tU7JMSnYYO9Urm9xryB

You can see the 'problem' by inspecting the DOM. The .cdk-overlay-container element still has stuff in it after you click 'Open dialog' -> 'Remove' -> 'Yes'. I guess it's up to you guys whether this is still an issue.

Also, I used NoopAnimationsModule. The problem doesn't occur with BrowserAnimationsModule because the dialogs take time to close.

@crisbeto
Copy link
Member

crisbeto commented May 4, 2017

Thanks, that's definitely a bug. I'll look into it.

@crisbeto
Copy link
Member

crisbeto commented May 8, 2017

The issue seems to be with the animations module @dbandstra. You can work around it either by using closeAll to close all of the dialogs, or wrapping the final dialogRef.close call in a setTimeout.

@angular-automatic-lock-bot
Copy link

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-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants