Support Volume Up/Down keypresses for amps that only support nudging volume

[ianparkinson]

For the CXNv2 in Control Bus mode, we can't control the volume numerically. Instead we can only send volume up/down commands to the streamer.

VibinUI has (in KeyboardShortcutsManager) support for controlling the volume using the up and down arrows. This presently uses the numeric volume setting, and so doesn't support configurations such as the CXNv2's Control Bus mode.

[ianparkinson]

There's a couple of different options here, and I'd like your opinion :)

We could add support for this situation to KeyboardShortcutManager, binding the up-arrow and down-arrow keys to the /api/system/amplifier/volume/up and .../volume/down actions. This poses a couple of problems:

1. In #261 we chose to use the -/+ symbols for the volume nudge buttons rendered in VibinUI. It's surprising to have up-arrow/down-arrow keys as shortcuts for the -/+ buttons. We could switch the keys to use -/+, or switch the buttons to use up-arrow/down-arrow - or just live with this inconsistency.
2. In this mode, we can't use the soft volume limit to reduce the risk of setting a dangerously high volume. The existing hotkeys retrigger if you hold the key down (that is, they trigger on the keypress event) which poses the risk that something holding the up-arrow key down could push the amplifier all the way up to its maximum volume very quickly, potentially damaging speakers. If we fix this, I'd like them to trigger on keydown instead of keypress to minimize this risk. I think this can be done by inspecting the KeyboardEvent passed to Mantine's HotkeyItem, but I haven't prototyped that yet. What do you think? I'd propose using keydown for volume control whether we're setting volume numerically or by nudges.

Alternatively, we could do nothing, and just leave this feature unsupported for such amps. Keyboard control via focusing the -/+ buttons works well enough.

[mjoblin]

We can always leave it as-is. That said, a few thoughts if we want to make changes (which I think would be great if time allows):

• Great idea about keydown vs. keypress. That sounds like an excellent change for sure. Excellent catch on that one.
  • Some people may still want to be able to hold the key down and have it continue to increase/decrease the volume, but at a slower rate. A debounce might allow for this -- although I'd still be wary of this causing problems. I don't have strong feelings about this though.
• I see your point about the -/+ buttons not being clearly associated with the down-arrow/up-arrow keys. The button choice was purely (on my part) a subjective aesthetic preference (which also happens to match some remote controls but not all). I don't feel strongly about this either. Another option might be to support both -/+ keys and the arrow keys; then we can keep the current buttons without them being misleading. (Although the "+" character technically requires the Shift key to be down... but personally I'd be fine with ignoring that oddity :) ).

Actually I just realized the arrow keys might interfere with scrolling. I mostly use the mouse so haven't noticed this; and I don't have it running right now to test (I broke my network a couple of days ago and need to fix it up 😔 ). I'll have more thoughts once I figure my network out and can actually try this stuff again. Apologies!

[mjoblin]

I just tested the volume-as-integer PRs and everything worked great! Unrelated to those PRs, my testing did actually confirm what I thought about the arrow hotkeys overriding the ability to scroll the window using the arrow keys. That's something I never noticed before (I'm always using the mouse). I'm OK with not letting that influence how we proceed here, but I wanted to mention it for completeness.

[ianparkinson]

Great, thanks!

A slight correction - I misremembered the difference between keydown and keypress (experimentally, we're getting repeated keydown events, but there's a repeat property on the event that can be used to disinguish between the two cases).

Good thinking about uparrow/downarrow being used to scroll the page. I'd propose changing to use the plus- and minus-keys instead, which fixes that issue and makes it consistent with the volume nudge buttons. The only browser action that I know of associated with plus and minus is zooming the browser, which (for Chrome, at least) is bound to ctrl-plus and ctrl-minus leaving us safe to use unmodified plus/minus as well as shift-plus/shift-minus for the larger steps.

[mjoblin]

Thanks for the keydown/keypress clarification. That makes good sense, as does using repeat. Thanks for investigating that.

And I agree about removing the arrow key hotkeys and replacing them with the plus/minus keys. Definitely sounds like a good approach.

[ianparkinson]

One problem - it looks like Mantine's useHotkeys system doesn't support listening for the + key. I've raised https://github.com/mantinedev/mantine/issues/7123.

(And I thought I sent this comment yesterday, apologies if I sent it to the wrong issue or something)

[mjoblin]

Good to know about the Mantine limitation. Thanks for following up on that. I'm happy to wait and see what happens there.

[mjoblin]

Looks like this has been fixed in Mantine 7.14.1, which is awesome. Unfortunately vibinui is still on Mantine v6, so benefiting from the fix might require migrating to v7. That's something which should be done at some point anyway, but it's likely non-trivial (I haven't looked for a while but I think it may require a fair number of changes to how styling is handled).

For future reference there's a migration guide at https://mantine.dev/guides/6x-to-7x/

That said, there might be other options available other than a full v7 migration.

[ianparkinson]

I've sent you pull request #278, which allows the existing up/down arrow keys to be used for volume nudging.

[ianparkinson]

A very quick prototype says that we could still switch to using the -/+ keys by bypassing Mantine's useHotkeys system for this and just adding an appropriate event listener to document.

Even with the fix to allow Mantine to support +, that might have a benefit. As well as the numpad '+/-' keys, US and UK keyboards (and presumably others) have a + bound to shift+=, and there's also a - key with _ bound to shift+-. Mantine uses KeyboardEvent.key which evaluates to the keyboard layout, but still requires the appropriate modifiers to match. So shift+[plus] would fire when the user types shift+=, but [plus] wouldn't fire when the user types =; and conversely - would fire when the user hits the minus key, but shift+- wouldn't when the user hits that combination. All of which is pretty ugly.

So we could switch to the -/+ keys and use our own event listener to restrict it to just the numpad keys by looking at the KeyboardEvent.code field instead of KeyboardEvent.key.

I'd say that's way too much complexity for a such minor feature where the arrow keys work well enough, so I'd propose leaving the keys as uparrow/downarrow. But it's not very much code and I'd be happy to add it in if you'd prefer.

[mjoblin]

Oh funny, my keyboard doesn't have a numeric keypad so I've only been thinking about the +/= and -/_ keys the entire time. I totally flaked on the numeric keypad!

I agree about the complexity and leaving things as they are with just the uparrow/downarrow keys. We can always revisit this in the future if necessary.

Thanks for testing the document approach though. Good to know that's an option if we ever need it.