RFR: 8090456: Focus Management [v8]

Sat Oct 12 15:21:17 UTC 2024

On Fri, 11 Oct 2024 17:40:45 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

> > You say a "bunch of event handlers" is less optimal; then optimize that. It's already there. In fact, a simple utility makes it a one-liner...
> 
> Let me know if I understood you correctly:
> 
> Let's consider an example where the application has a complex form and the product people asked for a keyboard navigation sequence that depend on let's say the form content. Maybe they have a grid of input fields and they want TAB key to either go down if the row is filled, or go right if the row is blank

It sounds like that's quite a high level facility they would require here, maybe even a special container (FormContainer, like FormLayout).  These relationships are probably best described not in terms of FX nodes, but probably with some kind of custom rule system.

> it does not matter, these requirements are not a subject of discussion as they came from the high up.

Yes, but what is subject of discussion is if, how and how specifically FX should cater to such use cases.  Is it sufficient to provide navigation methods to build a complex system on top?  If not, then what is missing that users absolutely can't do right now?  As it stands, this example is incredibly specific to business applications, and whatever FX provides here will likely fall short given that the rabbit hole of such applications goes very deep indeed.

FX is generalized, and caters all the way from simple one Window apps, to complex business applications, all the way to 3d apps, simulations and even (light) gaming.  The primary app I work on for example is nothing like a business application.  It doesn't have a single form or text box, does not use the mouse, and only uses the keyboard for navigation (and makes use of two level focus, a facility that has been considered for removal...).  It is nothing compared to another app I worked on that generated forms from XML descriptions, without ever instantiating a single control manually or touching things like FXML.

FX should provide the basic building blocks to allow for elaborate functionality like this, but we should not be providing a fully specialized solution.  For a similar reason I was not in favor of providing a full key mapping system (similar to what Eclipse provides) because that's a high level facility that FX should only cater for, not solve.

> So you propose to add an event handler to each input field where each event handler contains the necessary logic (dozens of those)? Am I right to assume that in this case the logic is all scattered across many event handlers creating testability issues and wasting oh so much memory on the event handlers (we have gigabytes, so who cares)?

That's one possible implementation.  If the fields are associated in some way with a rule system (with a property for example), the app can install the handlers automatically by going through the hierarchy (if it isn't plain generating the controls in the first place).  The handler would probably be the same each time (on key X, get focused node, get its id, check rule system, navigate to Y).

Another solution is to put this on the FormContainer (if there is such a thing, probably needed if you need such elaborate forms) or on some parent of the form.  Thanks to bubbled up events, you can use `getTarget` to find what was active (or use `Scene::focusedNode`) and apply the logic there in one place.  Given that controls currently just snatch events away from users (which they really shouldn't) you may be forced to do this with a filter instead of a handler.

Yet another option is to bind something conditional to `focusTraversable`.

This is the whole point of providing powerful features, the options are not limited to navigation or your imagination.  For example, what if we want to display an "Are you sure?" Dialog before navigation occurs?  That's all possible already.  The stories and issues from users complaining about focus problems so far can be categorized as follows:

- Plain ignorance
  - New to FX, where is feature X from toolkit Y?  FX is its own toolkit, build on very different premises. Just "copying" Swing/AWT/QT may not be a good fit at all.  FX has several powerful features that not every other toolkits has but new users may not be aware of (yet).  Often focus problems can be solved by doing one or a combination of:
    - Disabling/Enabling controls
    - Using functions like mouse transparent, focus traversable (which you could bind to an expression)
    - Re-arranging control order in certain containers (GridPane)
    - Event handlers
    - Dialogs
    - Custom containers (children are in kept in the logical navigation order, but displayed in a completely different order)

- Controls eating events
  - This is incredibly annoying for users.  When I add a "SPACE" event handler to a `Button`, I'd expect space presses to end up at my handler, yet internal code is using this **user** facility for its own purposes and runs first and consumes this event first without any recourse (aside from practically rebuilding the Control, which is what building a Skin entails).  Nowhere is this documented.  It just says "hey, you can add an event handler here, and handle any events you want", but in reality is that you can only handle events that internal code (again undocumented) deigns to leave for you.
  - Another example of this is the ScrollPane PR; it's eating keys, important keys, that users may want to repurpose but can't because there is no way to change ScrollPane's behavior, uninstall system handlers, or prioritize your handlers; in this case this behavior even interferes with default navigation that Scene handles when the keys remain unused

- API gaps
  - FX can clearly do logical navigation and directional navigation, but does not provide API to trigger these other than by faking a `KeyEvent`.  Where is my API to respond to `TextField` action's event with "navigate to next control"?

> Now we have a new requirement after a while where some fields get removed, some added, the logic changes, and now we need to modify that. So, following your suggestion, we just modify each handler with the new logic, carefully making sure that our changes are ok, right?

No, of course not.  The handler remains the same.  You don't have to hard code things in a handler, you can use standard Java constructs like `if`, map lookups, libraries that provide rule engines, etc.  You can associate properties and styles, and use those to determine your next course of action.

That's assuming of course that the act of adding/removing controls is not **already** providing the correct order (if I remove control Y that would be next from X, and then add a control Z after it, then X + Tab will navigate to Z).  

You already have much more control over navigation than you think.  Need a control skipped in certain cases?  How about **binding** its focus traversable to a **conditional** expression:

     controlThatNeedsSkipping.focusTraversableProperty().bind(ElaborateRuleEngine.get("rule-15.2").focusableProperty());

> Or maybe you want to add these handlers, but delegate the logic to a single class, at least one which can me easier to maintain? Still, a bunch of individual EHs but now we have a centralized facility that decides where to go depending on the form content?

And why not?  How many traversal policies and on how many controls would you need to install to do this solution? Can you prevent navigation?  Can you make navigation conditional on user input (a dialog)?  How would I make this system respond to completion of entering a form field?  I really don't want to press ENTER, then still need to press TAB.

> Are these scenarios describe your event handler suggestion?

Use the right tool for the job, FX provides more than event handlers.  In this case (a business form with elaborate rules) I expect users to build a rule system that they consult for the right action.  I'd probably generate the form (from a description), perhaps using a new container (FormContainer).  Controls can easily have handlers or properties bound when generated.

> (We are going to skip for the moment the case when skins change as a result of the user changing something in the UI preferences and all our EH fail due to https://bugs.openjdk.org/browse/JDK-8231245 )

Maybe we should fix that?  Because it sounds like justifying a feature because it **could** be done with event handlers, but its just unpredictable/broken right now.  

This is why a lot of my PR's focus on fixing **basic** problems (and I think the core developers should too).  This allows me and others to build reliable and elaborate systems **on** **top** of a stable, flexible and predictable foundation(*)

FX does not need to provide those, and can't, because beyond the basics of providing a UI framework lies the uncharted wilds of business applications that you can't possibly hope to cater for with tailor made solutions (the really heavy business apps probably generate forms -- I know I did when I was working on such apps, way too much effort to build forms in code or in FXML).

Is it really such a bad idea to take a step back, look into providing one part of this feature (unlocking navigation convenience methods) and then seeing if the problem is still so severe that more is needed?  I'm not the only one that is being critical here; I miss justifications for things like `NEXT_IN_LINE` and `TraversalMethod`.

I also miss the justification that exposing this internal API "as-is" is the best way forward (and no, limited developer capacity is only more of a reason to be conservative here and instead looking into fixing existing features).  Given that I still have seen only much hand waving, and few (if any) use cases that weren't asking for something out of ignorance or because framework X works that way, it may be good to hold off on that part until we're sure.

(*)
- stable - no memory leaks, no reentrancy bugs
- flexible - event handlers, listeners, custom controls, having a combinable and isolated triad of control+skin+behavior, map/flatMap, subscriptions
- predictable - controls not eating events, nested change events providing correct old values, resources being cleaned up immediately (less reliance on weak listeners), animation timing not being randomized, etc..

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1555#issuecomment-2408598755