RFR: 8090456: Focus Management [v8]

[Nir Lisker]

On Wed, 2 Oct 2024 17:33:15 GMT, Andy Goryachev <angorya at 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