RFR: 8091673: Public focus traversal API for use in custom controls [v3]

[Andy Goryachev]

On Sat, 26 Oct 2024 14:53:38 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review comments
>
> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 10442:
> 
>> 10440: 
>> 10441:     /**
>> 10442:      * Requests focus traversal from this {@code Node} in the specified direction.
> 
> I think that explaining what the method does by repeating the method name can be improved upon. I would use a simple "Requests to move the focus from...", or "Tries to move the focus from..." in case the method name doesn't end up using "try" and you want to use it in the docs instead.

thanks!

> modules/javafx.graphics/src/main/java/javafx/scene/Node.java line 10443:
> 
>> 10441:     /**
>> 10442:      * Requests focus traversal from this {@code Node} in the specified direction.
>> 10443:      * A successful traversal results in the newly focused {@code Node} visibly indicating its focused state.
> 
> A successful traversal also results in the newly focused node handling key events. Maybe add a bit of explanation on focus, like: "A focused node is the target of key events and has a visual indicator. A successful traversal results in a new {@code Node} being focused".
> Maybe even reverse the order of these sentences, this is just one example.

thanks!

> modules/javafx.graphics/src/main/java/javafx/scene/TraversalDirection.java line 33:
> 
>> 31:  */
>> 32: public enum TraversalDirection {
>> 33:     /** Moves focus downward. */
> 
> The direction itself doesn't move the focus. I would use a different phrasing, maybe something like "Indicates a focus change to the node below the currently focused node".
> 
> Same with the others.

good point!

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819347424
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819345925
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819339308