On Tue, 29 Oct 2024 13:01:08 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> This is better than the previous, since it only asks the caller of this 
>> method, typically the Skin of a custom control, to provide the one missing 
>> piece of information -- whether or not the method is in response to keyboard 
>> navigation -- and not ask the caller to compute the final answer. That's 
>> good. The only question I have is: can we do even better? Is there enough 
>> information for the implementation of `requestFocusTraversal` to know 
>> whether it was called from  keyboard event? If not, then this updated 
>> proposal seems reasonable.
>
> I think it could be possible, but I'm not sure if it would be pretty.  Scene 
> is the one dispatching these events, and since FX has a single threaded 
> model, Scene could have a flag that indicates it is currently dispatching a 
> key event.  If `requestFocusTraversal` is called, it can detect if this was 
> in response to a key event higher up the call stack by checking the flag on 
> Scene.
> 
> I do think there are other mechanisms in FX that pass along information from 
> shallower callstacks to deeper callstacks in some form or another.  
> `ExpressionHelper` for example is aware of nested calls and takes action 
> accordingly, and I'm almost certain there are more in places were FX is 
> dealing with peers and syncing the scene graph.

What happens when a different type of event is created by a `KeyEvent` handler? 
 How does the downstream code differentiate between the secondary events coming 
from a key event handler vs. from one originated normally?

> Is there enough information for the implementation of requestFocusTraversal 
> to know whether it was called from keyboard event?

The (original) intent of this API is to enable the custom components trigger 
the focus traversal as a response to key events only, the problem is that we 
can't prevent the application code from misusing the API and calling it for 
other reason.  (There is no other reason for the application code or custom 
component to call this method with the `visible=false`, other than misuse)

I would rather specify that this method must be called as a response to 
KeyEvent, and drop the argument.

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

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

Reply via email to