On Mon, 28 Oct 2024 23:53:44 GMT, Michael Strauß <[email protected]> wrote:
>> 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.
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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1820696736
