Re: Focus Traversal API for JDK 9

[Martin Sladecek]

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

Re: Focus Traversal API for JDK 9

[Jonathan Giles]

Thanks so much for chiming in here Martin! For those who don't know, focus 
traversal used to be one of Martin's many APIs that he used to own, so he knows 
it in great depth.

Martin, I'll parse and work through your sage advice as soon as possible. If 
you have any further advice for your old API, I am all ears.

Thanks a lot,
-- Jonathan
Sent from a touch device. Please excuse my brevity.

On 2 October 2015 19:04:20 GMT+13:00, Martin Sladecek 
 wrote:
>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

Focus Traversal API for JDK 9

[Jonathan Giles]

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