On Sat, 26 Oct 2024 14:53:38 GMT, Nir Lisker <nlis...@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
