Hi Andy,
First let me say that when it comes to designing an API, you really need
to take the time to think the solution through. The current internal
solution was probably kept internal for exactly that reason,
insufficient time to work out the kinks and look into alternatives.
An API is almost impossible to change later, so the general rule is that
if you're not sure about an API, then its better to have no API. This
is why I think it is important that we first look for what the API
should look like, then worry about how this can be fitted onto JavaFX.
Making concessions related to the current implementation before having a
clear idea of how the API should preferably work is not part of that.
You start making concessions only when it turns out the preferred design
would encounter unresolvable problems in the current implementation.
Since I think there is very little public API related to focus
traversal, nor is there any specification of how it currently works, I
think we have a lot of room to maneuver. This is why I think we should
first reach a consensus on the API, then look how it can be fitted on
top of FX. Sometimes a well thought out API also is a natural fit, and
may be easier to migrate to than you think.
On 14/09/2024 00:17, Andy Goryachev wrote:
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?
There's two things here.
1. There is no need to re-design the whole focus traversal. The old
internal system can be gradually replaced (it works by directly
consuming KeyEvents after all).
2. Regression. When nothing is specified, and the fact that controls
**ought** to work like other common controls in different UI toolkits,
is it a regression when focus traversal works the same as those other
platforms, even if it may be a regression from the point of view of FX?
For example, a Spinner will currently react to any mouse key, where as
other common toolkits only react to the left mouse button. Is it a
regression if FX is adjusted to also only react to the left mouse
button? It's not specified anywhere.
I think we have sufficient space to maneuver here as long as we are not
making focus traversal completely different from how it commonly works
in UI's.
Can there be regressions versus the current (unspecified)
implementation? Sure, there can be. Is that necessarily bad? That
depends. If the new focus traversal works like it does on all other
toolkits, then no, it is more of a bug fix. Did we break something with
the new implementation? That's always possible, but will then be fixed
as soon as it is reported.
#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.
I think TraversalEvents are quite central to making this an API that
will really stand the test of time. It leverages the existing event
system, giving you all the power that comes with it. You did not answer
my question about the TraversalEvents in your design. Why are the
Events when they can't be triggered, filtered or consumed?
#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.
Programmatic escapes can always be achieved by responding directly to a
TraversalEvent. I think however this should be a rare case, and
standard policies should really cover almost all use cases. It may be a
gap that should be investigated, and the API adjusted for (usually the
"exceptions" are well worth looking into to see if with a tweak they
can't become "standard"). As for being "strongly against" having two
properties -- that's an odd stance to take without motivating it. It
could also be rolled into "one" where the Policy is a record with the
two values, but I think we're getting ahead of ourselves here. First
the API, then the implementation.
I do think however there is great value in having the Logical and
Directional navigation split. Often you'll only want to replace one of
these with a custom policy (or a different standard policy), so that the
other navigation method can be used to escape the control. For example,
a Toolbar could be tabbed in an out of (using Logical navigation) while
the Directional navigation is cyclic (and thus can't be used to escape
the control's context).
#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.
I see no problem styling such properties. They're FX properties, and it
would be very convenient to style them to globally alter how focus
works, instead of having to rely on, say, Builders or Factories for
controls where traversal policies can be applied. There are also already
properties that don't only influence the look of controls. "-fx-skin"
being the most obvious one, but there is also "-fx-focus-traversable",
"-fx-context-menu-enabled", "-fx-block-increment", "-fx-unit-increment",
"-fx-pannable", "-fx-initial-delay", "-fx-repeat-delay",
"-fx-collapsible", "-fx-show-delay", "-fx-show-duration",
"-fx-hide-delay", and probably more. Aside from "-fx-skin" none of
these properties have a visual impact, but instead alter behavior.
Note: I'm not saying this needs to be there immediately. I just want to
make sure we're not closing off this direction, as again, it would be a
huge hassle to do this programmatically. In "code" the only things I
usually do on my controls are the following:
- I define the container hierarchy (VBox, HBox, which children go where)
- I set a style name
- I set anything that unfortunately cannot be CSS styled (things like
ALWAYS, SOMETIMES, NEVER grow policies, Grid sizes, etc, things that are
clearly "visual" but still can't be styled).
All the rest I don't touch, or want to touch. Having to select a
traversal policy for every control of type X I create is just cumbersome
and unnecessary. There will be a call then to set this "globally", and
then there will be the question, do we make something custom with many
limitations because it doesn't fit our conceptions of what (FX) CSS is
for (ie, not style, but only *visual* style) or do we just expose these
properties as Styleable leveraging an existing powerful system with
almost zero effort?
--
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.
"Quickly" and API's are incompatible with each other. There is nothing
worse than exposing an API quickly, which then becomes a burden on the
system -- I think the current CSS API is a prime example of where
"quickly" has gone wrong, costing us tremendous amounts of effort to
make even minor changes to it.
I urge you to ignore the current implementation, think thoroughly how
(in an ideal world) you would want such an API to work (from a user
perspective, not from an implementor's perspective) and only then see
how this could be made to fit into JavaFX.
This is exactly what I did. I did not look at the implementation,
although I'm aware of some of it. I looked at how I as a user of FX am
building applications, the struggles I have with it currently, (with
controls for example "eating" KeyEvents), and how I would like to be
able to adjust focus traversal. Do I want to respond to "KeyCode.LEFT"
or do I want to respond to "TraversalEvent.LEFT"? Do I also need to
respond to "KeyCode.NUM_PAD_LEFT"? These things should be abstracted,
and preferably I should just be able to choose from common navigation
standards. And when I do want to change such a standard, in 99% of the
cases that will be the case for all similar controls in my application.
How do I do such things currently if I want to change something for all
controls in my application? I use CSS.
Also I think this can be implemented gradually. Here's a potential plan:
1. Have Scene listen to unused KeyEvents and translate them to
TraversalEvents
Benefit: gives custom controls a way to respond to keyboard based
navigation in a platform agnostic way; this probably already removes the
biggest roadblock for custom controls...
Public API: Limited to a new Event
2. Start converting existing controls to listen to TraversalEvent
instead of KeyEvent
This hits a lot of controls, but should be relatively easy to do, and it
can be all kept internal for now. It can be done in a few batches.
Benefit: for each control converted, user can now programmatically
trigger focus changes, and by overriding things at Scene level can
completely change navigation keys
Public API: none
3. Implement a number of standard policies internally (OPEN, CONFINED,
CYCLIC, IGNORED)
Convert any controls that could use these as their default, removing any
custom logic if it happens to match one of the defaults.
Benefit: less code to maintain and debug, gives us experience with which
policies make sense and where the gaps are
Public API: none
Order: It is possible to do this before 2, and so some of the control
conversions could just consist of removing their custom logic, and
selecting a standard policy.
4. Expose policy property/properties on Parent
Any controls that are not using a custom policy anymore (of type
IGNORED) can now be user adjusted. We don't have to guarantee that each
policy makes sense for each control. Changing a default IGNORED policy
to a standard one will change the behavior (as intended) but it need not
be a "complete" behavior that users like. This is not FX's problem, and
can be improved upon later.
Benefit: users can now change policies on any existing control, even
ones with a custom policy; many of the controls may support a switch
between OPEN, CONFINED and CYCLIC out of the box.
Public API: new properties on Parent
5. Perhaps expose some helpful tools to calculate the "next" Node for a
given traversal option.
This can be done at any stage, and can be considered completely
separate. It is IMHO a relatively low priority need.
Benefit: less work for control implementors (although they could just
"copy" said code)
Public API: Maybe some methods in Node, or some kind of static helper.
6. CSS styleable properties
If we really want to give power to our users, and impress them with a
flexible focus traversal API, then make these properties styleable.
Benefit: allow users to pick any control, and set is policy globally or
within a subset of controls (ie. dialogs, popups, etc).
Public API: Nothing in Java, but document as CSS properties
--John
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
<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
