On Wed, 2 Oct 2024 17:33:15 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Public APIs for focus traversal and the focus traversal policy:
>> 
>> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/FocusTraversal/FocusTraversal.md
>> 
>> This work is loosely based on the patch
>> https://cr.openjdk.org/~jgiles/8061673/
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into 8090456.focus.traversal
>  - removed traversal event
>  - javadoc
>  - remove bounds
>  - whitespace
>  - Merge remote-tracking branch 'origin/master' into 8090456.focus.traversal
>  - Merge remote-tracking branch 'origin/master' into 8090456.focus.traversal
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8090456.focus.traversal
>  - review comments part 1
>  - ... and 4 more: https://git.openjdk.org/jfx/compare/4d3c3661...9e1fa796

Utility classes (static methods) decouple the behavior from the data and 
operate via side-effects. `Node` already defines `focusedProperty` (and 
`protected setFocused`), which means it has taken responsibility for focus 
management. This also means you can't ask for focus on `null` nodes.
Adding an external class makes it more difficult to do focus management. It 
also makes the code less clean because of the added utility class, which the 
user doesn't need because they already reference the object:


TextField tf = ...
tf.focusNext();


TextField tf = ...
ExternlUtility.focusNext(tf); // plus import


In general, this is anti-OO. While there are very valid cases for this sort of 
data-oriented design, it usually applies to processing of immutable data. 
Otherwise an object is usually responsible for mutating its own data and not 
let an external class do it.

I can just as well ask, why do we need all the `setOn__` methods on `Node`? We 
can have a utility class for `EventManager.setOn(Node node, Event event)`. Or 
any other property.
If `Node` is too big, create an internal `FocusManager` class (or with whatever 
suitable name) in its own file and have `Node` hold a reference to it while 
only delegating to its methods.

I have also been wondering (before this discussion) if it ever made sense for 
all nodes to have focus management. Many nodes are panes - can they actually be 
focused? If not, I would have used a `Focusable` interface only for those 
subclasses that can have focus.

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

PR Comment: https://git.openjdk.org/jfx/pull/1555#issuecomment-2403352472

Reply via email to