On Mon, 28 Oct 2024 18:07:17 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Maybe we should say that `Node.requestFocusTraversal()` should only be >> called by custom components/skins as a response to key press? I can't see a >> way to enforce that particular rule. >> >> Or perhaps `Node.focusVisible` doc should be updated to include the >> reference to `Node.requestFocusTraversal()` ? >> >> What's your opinion? > > 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? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1604#discussion_r1819561657