birchill / 10ten-ja-reader

A browser extension to translate Japanese by hovering over words.
https://addons.mozilla.org/firefox/addon/10ten-ja-reader/
GNU General Public License v3.0
588 stars 43 forks source link

Don't ignore Shift when it is pressed together with "hold to show" key(s) #658

Open birtles opened 3 years ago

birtles commented 3 years ago

Yet another keyboard issue I noticed: If you configure Ctrl as the "hold to show" key, while you are holding Ctrl you can't press Shift to cycle dictionaries. Instead, you need to first release Ctrl and then press Shift. That seems a little inconvenient. It's also inconsistent with the behavior for Enter.

I'm pretty sure it's due to the following code:

https://github.com/birchill/10ten-ja-reader/blob/c038d03ac0ea08d624385ea2d8a32c6f3036f1b2/src/content.ts#L318-L332

Note that pressing c doesn't work while Ctrl is being held due to the check for !ctrlKeyPressed here:

https://github.com/birchill/10ten-ja-reader/blob/c038d03ac0ea08d624385ea2d8a32c6f3036f1b2/src/content.ts#L430-L438

Similarly pressing d doesn't work since Ctrl + D in Firefox will try to create a bookmark.

Supporting Ctrl + Shift to toggle dictionaries seems like it should be possible and convenient, however.

birtles commented 3 years ago

It turns out this is tricky to fix without regressing #235.

That is, if Command is your hold-to-show key, and we allow pressing Shift while Command is held down to change dictionary, then as soon as you press Command + Shift we'll switch dictionaries and you won't be able to take a screenshot using Command + Shift + 3.

There's probably some heuristic we could use to work around this along the lines of introducing a "scanning mode". This would be entered as soon as we looked up a word based on a mousemove. We already do something similar to this for the "kanji lookup mode" and we could re-use the same flag for both features. Then we'd allow Shift + hold keys during scanning mode only and wouldn't break the screenshot use case, I think.

That's pretty complicated though so I'd like to hear if anyone else thinks this is a problem before I go ahead and fix it.