On Tue, 29 Oct 2024 12:23:59 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> How about this for `focusVisible`:
>>
>> /**
>> * Indicates whether this {@code Node} should visibly indicate focus.
>> * <p>
>> * This flag is set when the node is {@link #focusedProperty() focused}
>> via keyboard navigation,
>> * or when the input focus was programmatically moved from another node
>> to this node with
>> * {@link #requestFocusTraversal} while the other node visibly indicated
>> focus.
>> * <p>
>> * The {@code focusVisible} flag can be used by applications that do not
>> want a permanent focus
>> * indicator, and opt for a focus indicator that is only visible when it
>> is most helpful to the
>> * user. This is not the case when the user clicks on a control (because
>> they would know which
>> * control they clicked on), but only when a keyboard-initiated focus
>> traversal moves the input
>> * focus to another control.
>> */
>>
>>
>> As `requestFocusTraversal` is concerned, I think we can simply this a bit.
>> We don't need users to check whether `focusVisible` is set on the current
>> node; the implementation can do this internally. Here's a revised parameter
>> doc:
>>
>> * @param keyNavigation {@code true} if this method is called as a result of
>> keyboard navigation;
>> * {@code false} otherwise.
>>
>>
>> The implementation would then set `focusVisible` to true if either the
>> `keyNavigation` argument is true, or if the current node's `focusVisible`
>> flag is true.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1820751695
