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): don't close modal on ESC if file dialog is open #3455

Merged

Conversation

maxokorokov
Copy link
Member

@maxokorokov maxokorokov commented Nov 7, 2019

Keyboard events in modal and autoclose handling use 'keydown' instead of 'keyup'

Fixes #3439


Unfortunately no way of adding e2e tests for this case

Keyboard events in modal and autoclose handling use 'keydown' instead of 'keyup'

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

Codecov Report

Merging #3455 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3455   +/-   ##
======================================
  Coverage    91.1%   91.1%           
======================================
  Files          95      95           
  Lines        2789    2789           
  Branches      520     520           
======================================
  Hits         2541    2541           
  Misses        189     189           
  Partials       59      59
Flag Coverage Δ
#e2e 55.5% <100%> (ø) ⬆️
#unit 88.26% <100%> (+0.03%) ⬆️
Impacted Files Coverage Δ
src/util/autoclose.ts 84% <100%> (ø) ⬆️
src/modal/modal-window.ts 96.96% <100%> (ø) ⬆️

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 9a92667...c615f12. Read the comment docs.

Copy link
Contributor

@peterblazejewicz peterblazejewicz left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks!

src/util/autoclose.ts Show resolved Hide resolved
@peterblazejewicz
Copy link
Contributor

for reference #3415: same behaviour with this change (local tests, Catalina)

@maxokorokov maxokorokov merged commit 5977dcb into ng-bootstrap:master Nov 8, 2019
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.

Modal is dismissed with Esc when file dialog is open
3 participants