On Mon, 28 Oct 2024 18:35:23 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Let's consider a few scenarios:
>>
>> 1. A node has acquired focus via keyboard navigation. Its `focusVisible` bit
>> is set. When I press "tab", I expect the next node to also have visible
>> focus. This is the status quo.
>> 2. Same as 1, but instead of pressing "tab", I press "enter", which has a
>> custom handler that changes focus with the new `requestFocusTraversal` API.
>> I think is is quite natural to assume that the visible bit will also carry
>> over to the new node.
>> 3. A node acquires focus with mouse navigation. Its `focusVisible` bit is
>> not set. When pressing "tab", I expect the next node to have the visible bit.
>> 4. Same as 3, but instead of pressing "tab", I press "enter". The custom
>> handler changes focus programmatically, and the new node does not have the
>> visible bit (if we follow the current spec).
>>
>> It seems we have the same problem that we originally had with
>> `requestFocus()`, namely that we can't distinguish whether the API was
>> called in response to a key press, or in response to any other event (mouse,
>> programmatically, etc.).
>>
>> Back when `focusVisible` was added, we decided to always clear the visible
>> bit, because this seemed more correct than to always set it. Like you say,
>> there's no way to tell why someone called `requestFocusTraversal`, and thus
>> we can never clearly know whether to set or to clear the visible bit.
>>
>> Since we can't know, maybe the right course of action is to allow the caller
>> to specify whether to clear or set the visible bit, because the caller
>> should know that. The API could then be something like:
>> `requestFocusTraveral(TraversalDirection, boolean visible)`.
>
> I think the javadoc for `Node.focusVisible` (and its sibling
> `Node.focusWithin`) is rather insufficient. The JBS ticket is not a
> normative document, perhaps this should be clarified in a follow-up ticket
> (by moving some of the verbiage from JDK-8268225 ?)
>
> I am not against adding a boolean for `visible` (or `focusVisible` ?)
> argument.
>
> @mstr2 what would be a good description for this parameter?
What do you think of the following:
* @param visible Specifies whether the {@link #focusVisibleProperty()
focusVisible} flag will be
* set on the node that receives focus. Callers must specify
{@code true} if this
* method is called as a result of keyboard navigation, or if the
current node
* visibly indicates focus; in all other cases, callers must
specify {@code false}.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819601358
