Proposal: Focus Traversal API

[John Hendrikx]

Hi Andy / List,

I've given this some thought first, without looking too much at the 
proposal.

In my view, focus traversal should be implemented using events, and FX 
should provide standard handling of these events controlled with 
properties (potentially even CSS stylable for easy mass changing of the 
default navigation policy).

## KeyEvent and TraversalEvent separation

I think the cleanest implementation would be to implement a KeyEvent 
listener on Scene that takes any unused KeyEvents, checks if they're 
considered navigation keys, and converts these keys to a new type of 
event, the TraversalEvent. The TraversalEvent is then fired at the 
original target. The TraversalEvent is structured into Directional and 
Logical sub types, and has leaf types UP/DOWN/LEFT/RIGHT and 
NEXT/PREVIOUS.  Scene is the logical place to handle this as without a 
Scene there is no focus owner, and so there is no point in doing focus 
traversal.

This separation of KeyEvents into TraversalEvents achieves the following:

- User can decide to act on **any** key, even navigation keys, without 
the system interfering by consuming keys early, unexpectedly or even 
consuming these keys without doing anything (sometimes keys get consumed 
that don't actually change focus...).  The navigation keys have many 
possible dual purposes, and robbing the user of the opportunity to use 
them due to an overzealous component interpreting them as traversal keys 
is very annoying.  Dual purposes include for example cursor control in 
TextField/TextArea, Scrollbars, etc.  The user should have the same 
control here as these FX controls have.

- Scene is interpreting the KeyEvents, and this interpretation is now 
controllable.  If I want a Toolbar (or the whole application) to react 
to WASD navigation keys, then installing a KeyEvent handler at Scene 
level or at any intermediate Parent level that converts WASD to 
UP/LEFT/DOWN/RIGHT Traversal events would affect this change easily.

- The separation also allows to block Focus Traversal only, without 
blocking the actual Keys involved.  If I want to stop a Toolbar from 
reacting to LEFT/RIGHT, but I need those keys higher up in the 
hierarchy, then I'm screwed.  With the separation, the key events are 
unaffected, and I can block Toolbars from reacting specifically to 
traversal events only.

## Traversal Policy Properties on Parent

I think FX should provide several policies out of the box, based on 
common navigation patterns.  The goal here is to have policies in place 
that cover all use cases in current FX provided controls.  This will 
provide a good base that will cover probably all realistic work loads 
that custom controls may have. The goal is not to support every esoteric 
form of navigation, instead an escape hatch will be provided in the form 
of disabling the standard navigation.

In order to achieve this, I think Parent should get two new properties, 
which control how it will react to Directional and Logical navigation.  
These will have default values that allow navigation to flow from Node 
to Node within a Parent and from Parent to its Parent when navigation 
options in a chosen direction are exhausted within a Parent.  Custom 
controls like Combo boxes, Toolbars, Button groups, etc, can change the 
default provided by a Parent (similar to how some controls change the 
mouse transparent flag default).

These two properties should cover all realistic needs, and IMHO should 
be considered to be CSS stylable in the future to allow easy changing of 
default policies of controls (ie. want all Toolbars to react differently 
to navigation keys, then just style the appropriate property for all 
toolbars in one go).

Parent will use these properties to install an event handler that reacts 
to TraversalEvents (not KeyEvents).  This handler can be fully disabled, 
or overridden (using setOnTraversalEvent).

- logicalTraversalPolicy
- directionalTraversalPolicy

The properties can be set with a value from a TraversalPolicy enum.  I 
would suggest the following options:

- OPEN

This policy should be the default policy for all Parents.  It will act 
and consume a given TraversalEvent only when there is a suitable target 
within its hierarchy.  If there is no suitable target, or the target 
would remain unchanged, the event is NOT consumed and left to bubble up, 
allowing its parent(s) to act on it instead.

- CONFINED

This policy consumes all TraversalEvents, regardless of whether there is 
something to navigate to or not.  This policy is suitable for controls 
that have some kind of substructure that we don't want to accidentally 
exit with either Directional or Logical navigation.  In most cases, you 
only want to set one of the properties to CONFINED as otherwise there 
would be no keyboard supported way to exit your control.  This is a 
suitable policy for say button groups, toolbars, comboboxes, etc.

- CYCLIC

Similar to CONFINED but instead of stopping navigation at the controls 
logical boundaries, the navigation wraps around to the logical start.  
For example, when were positioned on the right most button in a button 
group, pressing RIGHT again would navigate to the left most button.

- IGNORED

This is similar to the mouseTransparent property, and basically leaves 
the TraversalEvent to bubble up.  This policy allows you to completely 
disable directional and/or logical navigation for a control.  Useful if 
you want to install your own handler (the escape hatch) but still want 
to keep either the default directional or logical navigation.

Possible other options for this enum could include a version that 
consumes all TraversalEvents (BLOCK) but I don't see a use for it at the 
moment.  There may also be variants of CONFINED and CYCLIC that make an 
exception for cases where there is only a single choice available.  A 
ButtonGroup for example may want to react differently depending on 
whether it has 0, 1 or more buttons. Whether these should be enshrined 
with a custom enum value, or perhaps a flag, or just left up to a custom 
implementation is something we'd need to decide still.

## Use Cases

1) User wants to change the behavior of a control from its default to 
something else (ie. a Control that is CYCLIC can be changed to OPEN or 
CONFINED)

Just call the setters with the appropriate preferred policy. This could 
be done in CSS for maximum convenience to enable a global change of all 
similar controls.

2) User wants to act on Traversal events that the standard policy leaves 
to bubble up

Just install a Traversal event handler either on the control or on its 
parent (depending on their needs).  A potential action to an unused 
Traversal event could be to close a Dialog/Toast popup, or a custom 
behavior like selecting the first/last item or next/previous row (ie. if 
I press "RIGHT" and there is no further right item, a user could decide 
to have this select the first item again in the current Row or the first 
item in the **next** Row).

3) User wants to do crazy custom navigation

Set both policies to IGNORED, then install your own event handler (or 
use the setOnTraversalHandler to completely override the handler).  Now 
react on the Traversal events, consuming them at will and changing focus 
to whatever control you desire.

4) User wants to change what keys are considered navigation keys

Install event handler on Scene (or any intermediate Parent) for 
KeyEvents, interpret WASD keys as UP/LEFT/DOWN/RIGHT and sent out a 
corresponding Traversal event

5) User wants to use keys that are considered navigation keys for their 
own purposes

Just install a KeyEvent handler as usual, without having to worry that 
Skins/Controls eat these events before you can get to them

6) User wants to stop a control from reacting to traversal events, 
without filtering navigation keys completely

With the separation of unconsumed KeyEvents into TraversalEvents, a user 
can now block only the latter to achieve this goal without having to 
blanket block certain KeyEvents.

-----

About the Proposal:

I think the Goals are fine as stated, although I think we differ on what 
the Traversal events signify.

I think CSS support should be considered a possible future goal. The 
proposal should therefore take into account that we may want to offer 
this in the future.

Motivation looks okay.

 > The focus traversal is provided by the FocusTraversal class which 
offers static methods for traversing focus in various directions, 
determined by the TraversalDirection enum.

I think these methods don't need to be exposed with a good selection of 
standard TraversalPolicy options.  After all, there are only so many 
ways that you can do a sensible navigation action without confusing the 
user, and therefore I think these policy options will cover 99% of the 
use cases already.  For the left over 1% we could **consider** providing 
these focus traversal functions as a separate public API, but I would 
have them return the Node they would suggest, and leave the final 
decision to call requestFocus up to the caller.  Initially however I 
think there is already more than enough power for custom implementations 
to listen to Traversal events and do their own custom navigation.  If it 
is not similar to one of the standard navigation options, the 
traverseUp/Down functions won't be of much use then anyway.

About your typical example:

     Node from = ...
     switch (((KeyEvent)event).getCode()) {
     case UP:
         FocusTraversal.traverse(from, TraversalDirection.UP, 
TraversalMethod.KEY);
         event.consume();
         break;
     case DOWN:
         // or use the convenience method
         FocusTraversal.traverseDown(from);
         event.consume();
         break;
     }

I think this is not a good way to deal with events.

1) The event is consumed regardless of the outcome of traverse. What if 
focus did not change?  Should the event be consumed?

2) This is consuming KeyEvents directly, robbing the user of the 
opportunity to act on keys considered "special" by FX.

3) This code is not only consuming KeyEvents directly, but also deciding 
what keys are navigation keys.

So I think this example code should be different. However, first I 
expect that in most cases, configuring a different traversal policy on 
your Parent subclass will already be sufficient in almost all cases 
(especially if we look at FX current controls and see if the suggested 
policies would cover those use cases).  So this code will almost never 
be needed.  However, in the event that you need something even more 
specific, you may consider handling Traversal events directly.  In which 
case the code should IMHO look something like this:

     Node from = ...

     Node result = switch(traversalEvent.getEventType()) {
       case TraversalEvent.UP -> FocusTraversals.findUp(from);
       case TraversalEvent.DOWN -> FocusTraversals.findDown(from);
       // etc
    }

     if (result != null) {
          result.requestFocus();
          traversalEvent.consume();
     }

Note that the above code leaves the final decision to call requestFocus 
up to the caller.  It also allows the caller to distinguish between the 
case where there is no suitable Node in the indicated direction and act 
accordingly.

This allows it to NOT consume the event if it prefers its Parent to 
handle it (if the control doesn't want CYCLIC or CONFINED style 
navigation).  It also allows it to further scrutinize the suggested 
Node, and if it decides it does not like it (due to some property or CSS 
style or whatever) it may follow up with another findXXX call or some 
other option to pick the Node it wants.  It also allows (in the case of 
no Node being found) to pick its own preferred Node in those cases.  In 
other words, it is just far more flexible.

I'm not sure yet where to place these static helper methods (if we 
decide to expose them at all initially), or even if they should be 
static.  Given that its first parameter is always a Node, a non-static 
location for them could simply be on Node itself, in which case the 
calling convention would become "Node result  = 
from.findTraversableUp()" (suggested name only)

 > Focus traversals generate a new type of event, encapsulated by the 
class TraversalEvent which extends javafx.event.Event, using the event 
type TraversalEvent.NODE_TRAVERSED.

What is the point of this event?  If you want to know that focus 
changed, you can add a listener to Scene.focusOwnerProperty.  What does 
it mean if I filter this event?  What if I consume it?  I don't think 
this should be an event at all, unless implemented as I suggested above, 
where consuming/filtering/bubbling can be used to control how controls 
will react to navigation events.

--John



On 03/09/2024 21:33, Andy Goryachev wrote:
>
> Dear fellow developers:
>
> I'd like to propose the public focus traversal API:
>
> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/FocusTraversal/FocusTraversal.md 
> <https://github.com/andy-goryachev-oracle/Test/blob/main/doc/FocusTraversal/FocusTraversal.md>
>
> Draft PR:
>
> https://github.com/openjdk/jfx/pull/1555
>
> Your comments and suggestions will be warmly accepted and appreciated.
>
> Thank you
>
> -andy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20240912/ff475260/attachment-0001.htm>