On Sat, 8 Feb 2025 10:35:00 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> This looks really good.  I'm wondering if this could be simplified further.  
>> Specifically, I think the `hoistFocus` flag and manual management of the 
>> focus delegate may not be needed.
>> 
>> It seems to me that a Control could share some similarities with a Scene, in 
>> that Control has properties that track a focus owner (similar to focus 
>> delegate). In effect, a Control is a focus root similar to scene.  When a 
>> Scene receives focus, it determines the best Node to "delegate" focus to; 
>> similarly, when a Control receives focus, it determines which skin control 
>> should be focused.  The normal focus rules should do the right thing here 
>> and for example select the TextField of a Spinner as the delegate 
>> automatically (some children may need to be marked as not focusable to guide 
>> the auto selection, but this is an already existing standard mechanism).
>> 
>> When determining where to send events, if the target is a focus root, it 
>> queries its focus owner (or focus delegate) and extends the event to that 
>> target.  If that target is also a focus root, the process repeats.
>> 
>> The request focus function should operate differently as well.  It should 
>> look for the closest focus root (a Control or Scene) and call the 
>> appropriate request focus function on the root it finds.  If that root is 
>> Scene, everything works as usual.  If it is another focus root like a 
>> Control, Control can determine the best way to focus one of its child nodes 
>> (likely you can just apply a normal search for an eligible focusable control 
>> for this).
>> 
>> Perhaps the focus root functionality can be captured in an interface that 
>> both Scene and Control implement.  I think it would need to specify a 
>> `requestFocus` method and `focusOwnerProperty`.  This interface would then 
>> replace the `focusScope` flag.
>
>> @hjohn Could you provide an outline of the algorithm that a control would 
>> use to automatically determine the focus delegate? I really can't envision 
>> what that would look like. For that matter I'm not sure why we would rely on 
>> an algorithm when in the few existing cases where focus delegation is needed 
>> the control knows exactly which node to delegate to.
> 
> Well, `Scene` determines the initial focus node by doing a depth first search 
> on its children and focusing the first child that is focus traversable and 
> enabled.  I don't see why this wouldn't work for controls exactly the same 
> way.  It's almost always the correct default, and in cases when it isn't, it 
> could still be overridden manually.  Take a ComboBox for example.  It 
> consists of a field and a button (or vice versa if you have a different 
> skin).  In either case however it would focus the field as the button in the 
> combo is not focus traversable.
> 
> Skins therefore can control the focus delegate by making appropriate use of 
> focus traversable and enabled, just like you can control what Scene would 
> pick for its initial focus.

> @hjohn I think what's missing in your model is the option to have an 
> independently focusable node inside of a control. For example, think of a 
> Button placed within a Button (via its `graphic` property). Clicking the 
> nested button should not transfer the focus to its parent button. Whether a 
> focus request should be hoisted is a choice that depends on the specifics of 
> the control, and we probably need an API for that.

Okay, agreed, so it would be good to have Nodes control whether or not the 
focus traverses up.  

I could imagine the flag could also in the future perhaps allow the Root Node 
or Scene to determine whether the Window should receive focus (perhaps special 
cases like popups or transparent overlays would allow interaction without 
hoisting focus to the Window?)

Then what about the focus delegate?  This is not a full FX property in this 
API, but seems very similar to `focusOwner` in Scene, in that it determines to 
which `Node` (keyboard) events are directed.  One could say that, since 
Window/Scene are also focusable, Scene is **delegating** events/focus to a 
node.  Would it be reasonable to have `focusDelegate` be a read-only property 
like in `Scene`, and perhaps rename it to focus owner (or if that's too 
drastic, ensure that `Scene` at some later stage --might-- implement the same 
interface and then duplicating its focus owner property as focus delegate)?

This can be achieved later, just interested in hearing your thoughts:
- A getter can later become a property
- A getter can later become part of an interface
- Scene can later implement an interface, even if it means duplicating the 
functionality of a property (if not named the same already)

I see some overlap, and having Nodes/Scene/Window implement a common interface 
(like `EventTarget`) is not unheard of.

Again, I love this proposal, and would like to move it forward.

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

PR Comment: https://git.openjdk.org/jfx/pull/1632#issuecomment-2646187233

Reply via email to