On Mon, 28 Oct 2024 23:20:49 GMT, Andy Goryachev <[email protected]> wrote:
>>> If this flag is suddenly so important, then why doesn't `requestFocus` have
>>> it?
>>
>> That's easy: because we didn't deem it necessary. Back when we added the
>> flag, only internal code would be able to set it, and that has worked out
>> fine. Focus traversal using keyboard navigation didn't use `requestFocus()`.
>>
>> Now we have a public API, and we face the same question again. There is a
>> clear advantage of setting the focusVisible flag as a parameter of
>> `requestFocusTraversal`: we ensure consistency between `focused` and
>> `focusVisible`, such that a node cannot be focusVisible if not focused.
>> Having focusVisible be a writable property would allow applications to have
>> a node visibly indicate focus, or even multiple independent nodes indicating
>> visible focus simultaneously, while yet another node is actually focused.
>> That's not good.
>
> Perhaps we need to further clarify what the focused/focusVisible/focusWithin
> properties are for?
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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819905489
