Hi Jonathan,
just a few thoughts about the API.
When I was adding some of the functionality to traversal, I could not
touch the public API. All we had there was the impl_ method for adding
TraversalEngine. This means there is some functionality that IMO should
have been directly in the scenegraph.
First of all, the TraversalEngine serves for 2 different purposes: 1)
overriding traversal algorithm for the given subtree 2) adding listener
to the subtree.
I think only the #1 should be really part of the TraversalEngine. It
would be better to register listeners directly on the scene/parent. This
would simplify the code as currently there are engines that are really
just a containers for listeners and they need to be special-cased.
The setOverriddenFocusTraversability/ isParentTraversable is there just
for Toggle button traversability ( RT-36659). I don't think this should
be part of the public API. This basically switches off the
traversability of unselected nodes that are part of a Toggle group, so
that only the selected one can be traversed to. The
ParentTraversalEngine is misused there just mark the node has overridden
traversability. I'm not sure how to replace it though. Too bad that
Toggle/ToggleGroup is in control and traversal engine does not know
about it, so it cannot check for it. This might be possibly marked by a
property (?) directly on the node? It would be kind-of a property
intended to be used by controls and not by the user, as the user already
has his ways to control the focus.
Anybody has a better idea here?
Also, TopMostTraversalEngine probably shouldn't be exposed, unless
there's public Scene property for it. This would allow to change the
default algorithm for the Scene (one that can be then accessed in
TraversalContext). The constructor for this is missing (still
package-private) however. If you want this functionality, you should
probably publish also (Sub)SceneTraversalEngine. If not,
TopMostTraversalEngine is useless and may stay in the private API space.
Cheers,
-Martin
On 2.10.2015 03:38, Jonathan Giles wrote:
Hi folks.
I'm continuing along with working on a few small API additions for JDK
9 to smooth off some of the rough edges due to Jigsaw modularity.
Today there is a relatively big API to discuss - focus traversal.
In summary, my plan is to essentially bring forward the focus
traversal API out of com.sun.javafx.scene.traversal and move it to
javafx.scene.traversal. I have done that on my machine, and along with
a few cleanups to reduce the API surface area and to clarify some
generic class names, it is ready for review. This isn't to say it is
final, or even guaranteed to go in to JDK 9, but I wanted to get your
thoughts on it so that we can move forward with the javadoc effort,
etc. The JavaDoc that you see presently is unchanged from what was
there previously, so a major task still to do is to fully document the
new API.
If it proves contentious, the likely plan would be to return to this
again after JavaOne, and if things can't be resolved it might not make
it into JDK 9.
I've uploaded the latest javadocs for this API to my website, which
you can see here:
http://jonathangiles.net/javafx/jdk9/traversal/
Any thoughts you have would be welcome!
Thanks,
-- Jonathan
