fix(dropdown): close dropdown menu on Enter / Space #3180
Conversation
maxokorokov
force-pushed
the
dropdown/fix/close-on-enter-esc
branch
from
0b8af1e
to
78da8cf
Compare
Codecov Report
@@ Coverage Diff @@
## master #3180 +/- ##
==========================================
+ Coverage 91.9% 91.91% +0.01%
==========================================
Files 91 91
Lines 3025 3030 +5
Branches 497 499 +2
==========================================
+ Hits 2780 2785 +5
Misses 179 179
Partials 66 66
Continue to review full report at Codecov.
|
| '(keydown.End)': 'dropdown.onKeyDown($event)' | ||
| '(keydown.End)': 'dropdown.onKeyDown($event)', | ||
| '(keydown.Enter)': 'dropdown.onKeyDown($event)', | ||
| '(keydown.Space)': 'dropdown.onKeyDown($event)' |
There was a problem hiding this comment.
I think it's time to keep a single (keydown) binding and create a switch or []. includes in a local onKeyDown($event) that will delegate to dropdown.onKeyDown($event)
WDYT ?
There was a problem hiding this comment.
@benouat, I agreed and I have a refactoring of keyboard handling in works. Would ideally just keep it separate from this PR
- you'll see a comment by @fbasso soon, but actually
NgbDropdownToggledoesn't need(keydown.Enter|Space)handlers at all. I'll fix that in a bit. - I'll split the
onKeyDowninto two functions forNgbDropdownToggleandNgbDropdownMenuI guess and it should simplify things.
P.S. also [].includes needs a polyfill :)
src/dropdown/dropdown.ts
Outdated
| @@ -109,7 +111,9 @@ export class NgbDropdownAnchor { | |||
| '(keydown.ArrowUp)': 'dropdown.onKeyDown($event)', | |||
| '(keydown.ArrowDown)': 'dropdown.onKeyDown($event)', | |||
| '(keydown.Home)': 'dropdown.onKeyDown($event)', | |||
| '(keydown.End)': 'dropdown.onKeyDown($event)' | |||
| '(keydown.End)': 'dropdown.onKeyDown($event)', | |||
There was a problem hiding this comment.
Handle this on the dropdown toggle doesn't seem to be necessary, as enter and space trigger the click (from Bootstrap specs, the toogle element is a button or a link only).
maxokorokov
force-pushed
the
dropdown/fix/close-on-enter-esc
branch
from
78da8cf
to
b6a7ee9
Compare
|
@fbasso thanks for the review:
|
Fixes #3142