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

Andy Goryachev angorya at openjdk.org
Mon Oct 28 16:35:19 UTC 2024


On Mon, 28 Oct 2024 16:21:25 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 10458:
> 
>> 10456:         case RIGHT -> TraversalDirectionInternal.RIGHT;
>> 10457:         case UP -> TraversalDirectionInternal.UP;
>> 10458:         default -> null;
> 
> Do we indent labels inside a switch? I find it more readable since `switch` opens a new context. This is also what I remember seeing in OpenJFX. Don't know if it's enforced.

I don't indent case labels inside of switch.
I don't think we enforce one way or the other.

And you can't ask to make it more compact and then complain that it's more compact /jk

> modules/javafx.graphics/src/main/java/javafx/scene/TraversalDirection.java line 40:
> 
>> 38:     /** Indicates a focus change to the next focusable Node, possibly traversing into the children of the current parent. */
>> 39:     NEXT,
>> 40:     /** Indicates a focus change to the previous focusable node. */
> 
> `NEXT` talks about traversing children, does `PREVIOUS` need to mention it too? Could it traverse out of the children of the current parent, or is it not symmetric?

I think the whole "children" thing can be dropped, since there is no NEXT_IN_LINE in the public API anymore.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819380921
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819374705


More information about the openjfx-dev mailing list