Dear John, Everyone:
Thank you for a thoughtful response! Some of the ideas you described
definitely deserve further consideration. If I were to summarize:
1. move the focus traversal logic away from the components and into the Scene
2. re-implement focus traversal through TraversalEvents rather than responding
directly to KeyEvents
3. (more) standard policies
4. using CSS
(there is of course more topics in your response, but let me start with the 4
above)
#1
I generally like this idea. In some sense it is already how things work
internally, but without the ability to customize that (i.e. by introducing
custom traversal keys, or removing existing ones). The downside is
substantial: not only we'd need to re-design the whole of the focus traversal,
but also rework the existing control's behaviors. Did I mention the risk of
regression, given the absence of comprehensive behavioral tests?
#2
This may or may not be an integral part of #1. Potentially, it allows for
injection of events by the application code, as well as simplifies creation of
complex custom controls. The latter becomes possible with the original
proposal, so net benefit is limited to the first part, I think.
#3
One obvious possibility is to enable creation of a simple policy based on a
list of Nodes. I must mention one use case that is impossible to cover with
pre-defined policy is one where navigation depends on some state. Such a
policy must be implemented programmatically. I think one property should be
sufficient - I am strongly against adding two properties here.
#4
The idea of using CSS to specify traversal policy seems wrong to me: the CSS
defines the presentation aspects (styles) rather than behavioral ones. I know
it is possible to set custom skins and the corresponding behavior via CSS, and
we know why (skins define the appearance), but we should not go beyond that, in
my opinion.
--
There is one more aspect of the problem that I think we should consider. The
current proposal does not change the implementation in any material way, nor
does it change the behavior, thus can be done quickly. The benefit everyone
gets from it is ability to trigger focus traversal and to control it via custom
policies. Any other solution will require resources and the bandwidth we
currently don't have, which means the probability of it being added to FX is
virtually zero. Let me emphasize, I am not against attempting to discuss or
implement the best possible solution, but we should be aware of the limitations
of the reality we live in.
So I would like to ask the following two questions:
- are there requirements that will be still impossible to deliver with this
proposal and how important they are?
- is there anything in this proposal that will make some important feature
impossible?
What do you think?
Thank you,
-andy
From: openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of John Hendrikx
<john.hendr...@gmail.com>
Date: Wednesday, September 11, 2024 at 19:05
To: openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
Subject: Re: Proposal: Focus Traversal API
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
Draft PR:
https://github.com/openjdk/jfx/pull/1555
Your comments and suggestions will be warmly accepted and appreciated.
Thank you
-andy
