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

fix(theme-shared):fix #2107,when use Esc key to close modal ,should trigger destroy to avoid raise an error #2130

Closed
wants to merge 1 commit into from

Conversation

yinchang0626
Copy link
Contributor

fix #2107
when use Esc key to close modal ,should trigger destroy to avoid raise an error

@mehmet-erim
Copy link
Contributor

mehmet-erim commented Nov 11, 2019

You should add in the visible setter. It's enough and correct way

After your changes, 46th - 49th lines should be like this:

else {
 this.renderer.removeClass(document.body, 'modal-open');
 this.disappear.emit();
 this.destroy.next();
 this.destroy.complete();
}

@yinchang0626
Copy link
Contributor Author

@mehmet-erim
thank for helping, i think you are right
I tried that way you say to fix,
when i test that code:
move code to visible setter is no problem,
but it appear error in this:

else {
 this.renderer.removeClass(document.body, 'modal-open');
 this.disappear.emit();
 this.destroy$.next();
 this.destroy$.complete();
}

and work in this

else {
 this.renderer.removeClass(document.body, 'modal-open');
 this.disappear.emit();
 this.destroy$.next();
}

I dont know why after call complete()
takeUntil(this.destroy$) will still triger the keyup event
if you has suggest ,pls let me know,
I will change it to

else {
 this.renderer.removeClass(document.body, 'modal-open');
 this.disappear.emit();
 this.destroy$.next();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(ng.theme.shared) get error when modal close
3 participants