Compare sheet won't close on esc if input focused

[duckmanBR]

What version of DIM are you using?
Beta 6.40.0.9199

What Browser and OS are you using?
Chrome 86.0.4240.198 / Win10

Describe the bug and how to make it happen
On compare, if a button or combo is selected/focused, esc won't dismiss/close it. Only unfocusing (clicking on an empty area) that it'll work again.

Include devtools console logs
Nothing specifc to the issue.

[liamdebeasi]

Hey there! I tested this on the latest version of DIM on Chrome 125/Windows 11, and I cannot reproduce the issue. When pressing "Escape" the sheet will close even if a button inside of the sheet is focused. Are you still able to reproduce the issue?

[bhollis]

Thanks for taking a look. The specific repro I had was on Windows and involved focusing a text field (e.g. the title of a loadout).

[liamdebeasi]

Thanks for the additional info! I'm able to reproduce this issue on both Windows and macOS. It looks like this check is the culprit. The issue is "fixed" if I remove that check, but it looks like this logic was implemented for a particular reason.

Do you have any additional context as to why this check is here? I'd be happy to make a PR if you have suggestions for how to correctly fix this.

[bhollis]

Yeah that code was brought over from Mousetrap. My understanding is that it's meant to avoid overriding builtin input hotkeys (including typing letters) with your custom hotkeys.

[liamdebeasi]

Thanks for the added info! I looked on the original mousetrap repo and found an issue that seems related. Adding a mousetrap class on to the input bypasses this behavior and allows keyboard shortcuts to fire when the input is focused.

My knowledge of the DIM codebase is still fairly limited, but I can think of a few possible ways of solving this issue:

1. Implement a similar .mousetrap class opt-out
   The benefit here is we can selectively apply this to specific options. However, this option would allow any keyboard shortcut to fire when the input is focused which may not be desirable.

2. Bypass the check in question if the key pressed matches an allowed list of keys
   The benefit of this approach is you could expand this over time if you find there are other hotkeys you'd like to allow while inputs are focused. One risk is this would apply to any input/textarea/select.

3. Some combination of 1 and 2
   This gives you the best of both worlds: selectively applying the opt-out to inputs and only for specific keys. However, this usage may be somewhat confusing since you'd need to maintain support in 2 places (one on the input for the class and one on the hotkey for the allowlist).

Is there a particular approach you'd prefer?

[bhollis]

Probably number 2 - esc should bubble out of the input. However, it's interesting that this isn't a problem on macOS.

[liamdebeasi]

Thanks! I opened a PR here: #10478