New properties: focusOwner and focusOwnerWithin

[John Hendrikx]

I don't see how a method like `setFocused` could possibly work without 
violating its specification.  When a Scene is present, it could in 
theory update the focusOwner, but when no Scene is present it probably 
needs to remain `false` even when set to `true` (as otherwise you could 
set multiple nodes to focused, unless you did a full scan of the entire 
UI hierarchy to avoid that...).

Perhaps it is possible to make setFocused do something useful; let it 
immediately change focus/focusOwner -- that could be tough as I think 
focus changes can be veto'd; the purpose of requestFocus would then only 
be to also focus the window.  The setFocused method would then be a 
useful feature.  It would still need a Scene to work (but its 
specification agrees with that part, so it can indeed remain false if 
not attached to a scene).  All nodes would need to clear focus status 
when the Scene is null'd, and when calling setFocused on a Node, any 
currently focused node in the same Scene needs to lose it (probably best 
to remove focus first so those notifications happen before the new node 
gains focus).

`setFocused(false)` would also need some special treatment; either it's 
not allowed at all, or some other node gets picked as the new focus 
owner (perhaps it switches to the first focusable node, sort of like a 
focus reset as if the UI was freshly created).

If the above ain't realistically possible, then I guess deprecation is 
the only option.

--John


On 11/10/2023 22:55, Michael Strauß wrote:
> I think the cleanest solution would be to have the implementation of
> Node.focused match its specification:
>
>      Indicates whether this {@code Node} currently has the input focus.
>      To have the input focus, a node must be the {@code Scene}'s focus
>      owner, and the scene must be in a {@code Stage} that is visible
>      and active.
>
> Obviously, the Node.focused property is not implemented and used as
> specified. But the problem is that Node.setFocused(boolean) is public
> API. If we wanted the implementation to conform to its specification,
> we would probably need to deprecate this API, and also make it
> ineffective so that user code cannot change the focused property. Then
> we need to fix all controls that use this API.
>
> Maybe we can also just fix the controls without removing the
> Node.setFocused API. But it begs the question why we would keep around
> an API that is only useful in circumventing the specification of a
> property. What do you think?
>
>
> On Sun, Oct 8, 2023 at 10:11 AM John Hendrikx <john.hendrikx at gmail.com> wrote:
>> I would be interested to know what use cases for these are, and what
>> would be then left of the use cases for the existing focusWithin and
>> focusVisible properties?  The ticket referred doesn't give one and only
>> reports a bug.
>>
>> Before implementing a second set of these types of properties, I would
>> like to know:
>>
>> - What would be the distinct uses for focusOwnerWithin vs focusWithin,
>> are both actually needed?
>> - Isn't focusOwnerProperty basically a "better" focusedProperty now
>> because of how focusedProperty is abused by some controls?
>> - Why indeed is TableView doing this and should it really be doing this
>> instead of doing this another way?  If it just wants to show nodes in
>> their focused style, that can (and should IMHO) be achieved differently.
>>
>> There seems to be a need to have some controls retain their focused look
>> without them actually being the focus owner. TableView seems to be one
>> of them, and I personally have another (I would like Nodes to keep
>> looking focused even when their Stage has lost focus). The focused look
>> relies on the `focused` pseudo class, which is now strictly controlled
>> by the focusedProperty. It may be a good idea to look in another
>> direction; one could change the style sheet for example to also have
>> controls in focused look when they have a different pseudo class, say
>> `fake-focused`.  TableView could then use this (and so could I).
>>
>> With the TableView problem solved, the original focusWithin,
>> focusVisible and focused properties then can serve the purposes they
>> were intended for, instead of basically replacing them with a new set
>> that acts slightly differently to work around a problem with TableView.
>>
>> If this does move forward though, I wonder why these need to be
>> properties specifically, given how hesistant we were to add properties
>> to Node?  The `shownProperty` which IMHO has far more uses given the
>> essential role it can play in deterministic clean-up was rejected over
>> such an argument.  Why isn't simply checking the psuedoClassState
>> sufficient for these properties:
>> `getPseudoClassStates().contains(FOCUS_OWNER_PSEUDOCLASS_STATE)`?
>>
>> --John
>>