commit 410f5224801d84425ef0270df0ca0055dd78bbeb
author Ben Rohlfs <brohlfs@google.com> Tue Oct 19 23:44:02 2021 +0200
committer Ben Rohlfs <brohlfs@google.com> Thu Oct 21 16:28:13 2021 +0200
tree 4cb467b35f7861d4b2d6d92d6522bb3860ec7d4e
parent 771d18a0162fd37f3fae010eeb7a569e57cfb271

Move most of shortcut mixin features into service

Make the keyboard shortcut mixin (and thus all of Gerrit) independent
from IronA11yKeysBehavior. The major change needed for this was
introducing `addShortcut` to the ShortcutsService as a replacement for
`addOwnKeyBinding`.

The views implementing `keyboardShortcuts()` needed to be changed to
return a proper callback instead of just a string.

We don't use `keyUp` events anymore. Everything is just `keyDown`.
`keyUp` was only introduced because of troubles handling combo keys,
which is now properly dealt with.

Along with `IronA11yKeysBehavior` the `IronKeyboardEvent` is also gone.
All code is just using the standard `KeyboardEvent` instead. Also the
`keyEventTarget` does not have any meaning anymore. The service adds
all listeners to `document.body` by default. If the need arises, then
we can introduce an option later for allowing shortcuts to also be
registered on a more "local" level.

The hundreds of `modifierPressed` could be removed, because of how
`eventMatchesShortcut` is implemented. If the modifiers are not matched
properly, then the shortcut will not be triggered. That also means that
some `shiftKey` checks could be removed.

We are generally suppressing `repeat` keydown events. If processing them
is desired, then we can bring them back as an explicit option. But
ignoring them seems like the best default.

If we are triggering a shortcut callback, then we are always calling
`preventDefault()` and `stopPropagation()` on the keyboard event. That
also seems a great default and allows to remove tons of individual
`preventDefault()` calls. If that turns out to be undesired in specific
scenarios, then we can introduce a dedicated option.

We are also always checking `shouldSuppress()` for every shortcut, so
the tons of individual checks could also be removed.

Some handlers were handling lowercase and uppercase chars in one method,
e.g. 'n' and 'N'. We have split those up in two dedicated handlers.

The combo key handling could be completely moved into the service. Those
special shortcuts are treated as normal shortcuts, but when checking a
shortcut against an event we also check whether the current combo state
matches the shortcut definition. This simplifies matters a lot.

Instead of calling addShortcut() directly for every shortcut, the mixin
just passes all information into `attachHost`, and then the service can
iterate over all the shortcuts and bindings. And thus also be
responsible for the cleanups (i.e. calling removeEventListener).

There are still a lot of potential follow-up cleanups and refactorings
that are staring into your face, but obviously this change is already
fairly large, so let's leave them for later. The major accomplishment
here is that replicating the mixin's functionality in a Lit controller
has become trivial. So the Lit migration of Polymer elements that
depend on the keyboard mixin is unblocked. Also note that the entire
"observer" related code in the mixin is something that we probably want
to get rid of altogether and replace it by something clearer and
simpler, e.g. the service knowing which view is current.

Change-Id: I5cf82feb1c8af16c579eab2120381c85e9607969