search

sparksuite / react-accessible-dropdown-menu-hook

A simple Hook for creating fully accessible dropdown menus in React
http://sparksuite.github.io/react-accessible-dropdown-menu-hook
MIT License
112 stars 26 forks source link

Fix listener added and removed wrong order #293

Closed corymharper closed 2 years ago

corymharper commented 2 years ago

This pull request adds a fix for event listeners that were being added and removed in the wrong order.

Closes #292

codecov[bot] commented 2 years ago

Codecov Report

Merging #293 (65ca9b2) into master (8f8c123) will decrease coverage by 1.48%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #293      +/-   ##
==========================================
- Coverage   90.90%   89.42%   -1.49%     
==========================================
  Files           1        1              
  Lines          99      104       +5     
  Branches       32       33       +1     
==========================================
+ Hits           90       93       +3     
- Misses          7        8       +1     
- Partials        2        3       +1
Impacted Files Coverage Δ
src/use-dropdown-menu.ts 89.42% <66.66%> (-1.49%) :arrow_down:

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 8f8c123...65ca9b2. Read the comment docs.

WesCossick commented 2 years ago

I'm assuming this solution works reliably? With both the physical escape key and the virtual one on the touch bar?

corymharper commented 2 years ago

I'm assuming this solution works reliably? With both the physical escape key and the virtual one on the touch bar?

Yes, the virtual escape key was not a specialized case but rather just an author catching the problem with a very specific scenario, the virtual escape key just fires the same event as the physical escape key. Besides that, I was not able to reproduce the bug after these changes, and it makes sense, with run to completion in mind, this approach will always detect if removal has happened before addition.