Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Keyboard toggle/icon and make key/mouse focus mutually exclusive #102

Merged
merged 7 commits into from
Apr 11, 2024

Conversation

sudara
Copy link
Owner

@sudara sudara commented Mar 17, 2024

This PR

Sine Machine v14 - 2024-03-17 57@2x

According to @baconpaul's comment here and my own futzing about, I'm making the assumption that when keyboard focus is enabled, mouse listening should be disabled. Everything feels too messy otherwise.

Note: Even with the overlay's mouse listening disabled, clicking on elements in JUCE UIs still does set keyboard focus by default, see https://docs.juce.com/master/classComponent.html#a5e27530ab343f52b524c1c3f1a1d98eb — in other words, most of the time focus will still change by clicking around the UI!

Questions:

  • Is the state of the toggle worth storing in settings->props? Or is it fine to always assume it should be off?
  • It would be nice to have some labels/tooltips on icons, people are going to be like wtf is that

@sudara
Copy link
Owner Author

sudara commented Mar 17, 2024

Ah, I didn't catch that the JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE code was added to an autogenerated file in #100. I'll have to add that to the ruby file that generates assets for it to persist...

@baconpaul
Copy link
Collaborator

Ahh sorry didn’t realize that was auto generated!

this change looks great. Having this choice be sticky in props would be lovely but I can also have sure menus launch different configs.

@sudara
Copy link
Owner Author

sudara commented Mar 18, 2024

A couple UX annoyances I'd like to fix on this branch.

  1. If I click to select a component in the UI and then go to click the keyboard focus toggle, the component loses selection. More specifically, the component loses selection anytime you click back to the inspector or to another app and doesn't regain it.

  2. I'm still sometimes seeing the hover behavior still active even when FOLLOW_FOCUS is on.

  3. the key focus listener has to be removed before inspector is closed (in destructor)

Not totally sure this is the right move, but going to look into filtering the global focus callbacks to only apply those related to the UI's component peer.

@sudara
Copy link
Owner Author

sudara commented Mar 19, 2024

Ok, that was a bit annoying to figure out.

Part of the "mutually exclusive" complication was that toggle was on a delayed timer — so on toggle, it would re-enable mouse listening to prevent some other UX glitchyness from happening.

The keyboard focus setting will now also be remembered.

I think I've cleanup up all the edge cases, with #104 as a bonus.

There are certainly some unideal things, like I don't feel 100% rock solid about the mouse listener enable/disable RAII situation, but I added a jassert to hopefully catch any issues bubbling up.

Going to live with this a bit longer before merging. Feel free to check it out too.

@sudara
Copy link
Owner Author

sudara commented Mar 19, 2024

There's still one issue I'm not sure exists / how to resolve / if I'm just lacking some understanding.

setRoot currently adds a keyListener and setsWantsKeyboardFocus of the root element to true. It does this to listen for some keyboard shortcuts. Which I think doesn't work.

I'm wondering if setsWantsKeyboardFocus is undesirable — for me, when I toggle on key focus, it focuses on the root element. Not only is that hard to see (see #61) but I'm wondering if the focus should actually be given away (to some child) in my app.

This should be reproducible:

  • Turn on keyboard focus
  • Select an element in the UI, like a text field
  • Click back to the inspector
  • Click back to the root plugin UI and focus on that child is lost, it's on the overall plugin

I think what's happening is that the root plugin UI is stealing focus, maybe incorrectly because of code in the inspector.

Where my understanding stalls out: I'd like to have some global keyboard shortcuts in my plugin. Does that mean setsWantsKeyboardFocus has to be true? It seems "bad" that a side effect of wanting shortcuts.

@sudara
Copy link
Owner Author

sudara commented Mar 20, 2024

Ignoring globalFocusChanged calls from root/inspector just traded one glitchy feel for another, for example:

  • Turn on keyboard focus
  • Select a component that can be hidden (for example text box or control in a modal or tabbed container)
  • Hide the component
  • The inspector selection is still there (which is wrong/glitchy)

Reverting back to just ignoring nullptr focus events, otherwise letting them through.

@sudara sudara merged commit 7ea5bec into main Apr 11, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants