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(modal): remove key/mouse handlers only when modal is destroyed #3532

Merged

Conversation

maxokorokov
Copy link
Member

@maxokorokov maxokorokov commented Jan 7, 2020

At the moment key/mouse handlers are removed when modal triggers dismiss event. The problem is that modal closure could be cancelled with beforeDismiss and modal will stay opened, but handlers will be removed.

This commit introduces closed$ event triggered inside onDestroy to clean up handlers.

Fixes #3515

@maxokorokov
Copy link
Member Author

@benouat could you PTAL?

At the moment key/mouse handlers are removed when modal triggers dismiss event. The problem is that modal closure could be cancelled with `beforeDismiss` and modal will stay opened, but handlers will be removed.

This commit introduces `closed$` event triggered inside `onDestroy` to clean up handlers.

Fixes ng-bootstrap#3515
@codecov-io
Copy link

Codecov Report

Merging #3532 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3532      +/-   ##
==========================================
+ Coverage   91.41%   91.42%   +<.01%     
==========================================
  Files          96       96              
  Lines        2807     2809       +2     
  Branches      518      518              
==========================================
+ Hits         2566     2568       +2     
  Misses        183      183              
  Partials       58       58
Flag Coverage Δ
#e2e 56.74% <100%> (+0.17%) ⬆️
#unit 88.17% <60%> (ø) ⬆️
Impacted Files Coverage Δ
src/modal/modal-window.ts 93.33% <100%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e5fac4...1c02d2c. Read the comment docs.

@maxokorokov maxokorokov merged commit 2b3ceed into ng-bootstrap:master Jan 7, 2020
maxokorokov added a commit that referenced this pull request Jan 7, 2020
…3532)

At the moment key/mouse handlers are removed when modal triggers dismiss event. The problem is that modal closure could be cancelled with `beforeDismiss` and modal will stay opened, but handlers will be removed.

This commit introduces `closed$` event triggered inside `onDestroy` to clean up handlers.

Fixes #3515
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.

Dismissing one of stacked modals dismisses keyboard/backdrop events for all modals
3 participants