Updated the API and the JEP per feedback comments: https://github.com/andy-goryachev-oracle/Test/blob/main/doc/FocusTraversal/FocusTraversal-v3.md
https://github.com/openjdk/jfx/pull/1604 Thanks, -andy From: Andy Goryachev <andy.goryac...@oracle.com> Date: Tuesday, October 22, 2024 at 12:22 To: John Hendrikx <john.hendr...@gmail.com>, openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> Subject: Re: Proposal: Focus Traversal API Thank you. I completely disagree with the idea of adding a single method to Node and introducing 6 way enum, for the reasons provided earlier. Having said that, I am more interested in moving forward. If everyone feels more comfortable with Node + enum combo, I'll make the change. Thank you for a lively discussion. -andy From: openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of John Hendrikx <john.hendr...@gmail.com> Date: Monday, October 21, 2024 at 23:46 To: openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> Subject: Re: Proposal: Focus Traversal API I already like this suggestion very much, as it is simple, concise and located exactly where you'd expect it (near `requestFocus`). It offers the navigation options that FX offers in a programmatic way and doesn't need to cater to other methods, only what FX offers. I also have an alternative approach that could be considered. The user model would be something like this: LogicalNavigation.NEXT.find(currentNode).ifPresent(Node::requestFocus); This is more wieldy, but does offer a natural API that allows extension as `LogicalNavigation.NEXT` is an implementation of an interface (provisional name Navigator): interface Navigator { Optional<Node> find(Node node); // finds a next node given a starting point } Then implementations of these can be made available as constants or enums, for example: enum LogicalNavigation implements Navigator { NEXT(InternalImplementation::next), PREVIOUS( ... ); ... } The above API offers a natural way to only "predict" the focus, and only act on it if you want. --John On 21/10/2024 19:13, Michael Strauß wrote: > I think it is a good idea to do the non-controversial part of this > feature first. > > With regards to the API, I would prefer a single instance method on Node: > > Node moveFocus(FocusTraversalKind); > > Where FocusTraversalKind is an enum with values UP, DOWN, LEFT, RIGHT, > NEXT, and PREVIOUS. > Note that it is not called "TraversalDirection", because NEXT and > PREVIOUS are logical types of navigation, not directional ones. > The return value is the node that will receive focus as a result of > this method call. > > The name "moveFocus" pairs with "requestFocus" (and we could also > potentially add "predictFocus" to get the node that would receive > focus, without actully changing focus). > > Having a single instance method allows for easier composition, as I > can now pass the FocusTraversalKind to methods, which is a bit awkward > to do with method references. > > For example, I could write a method: > > /* > * Moves the focus if allowed. > * Returns true if the focus changed, false otherwise > */ > boolean moveFocusIfAllowedByBusinessLogic(Node node, > FocusTraversalKind kind) { > return switch (kind) { > case UP, DOWN -> false; > default -> isAllowed() && node.moveFocus(kind) != node; > }; > } > > If I would instead pass a method reference to > FocusTraversal::traverseNext into my custom method, I'm very limited > in what I can do with it. > For example, I can't switch() on the method reference, I can't change > it with custom logic, etc. > > > On Fri, Oct 18, 2024 at 10:25 PM Andy Goryachev > <andy.goryac...@oracle.com> wrote: >> Dear fellow developers: >> >> >> >> Thank you for providing a lively discussion here in the mailing list, and in >> the PR. >> >> >> >> A surprisingly large number of objection was voiced, among them in no >> particular order (please correct me if I missed anything): >> >> >> >> - no need for the traversal policy API, developers have all the tools they >> need >> >> - rather than providing Parent.traversalPolicy, the whole thing should be >> delegated to Scene >> >> - no need for custom traversal policies, provide a number of standard >> policies (e.g. cyclic/logical; or via enum if possible) >> >> - introducing a new category of traversal events (later deemed unnecessary) >> >> - adding two properties to Parent for directional and logical traversal >> >> - desire to be able to configure traversal policy via CSS >> >> - focus traversal methods should be instance methods in the Node class >> >> >> >> Also, the discussion reiterated some of the design problems we have in >> JavaFX, namely >> >> >> >> - controls consuming key events that effect no change in the control's state >> >> - interference between controls' key handling and Scene's accelerators >> >> - undetermined priority of event handlers registered by the skin and the >> application >> >> >> >> I still believe there is a value in providing the traversal policy APIs, but >> given the general sentiment in the feedback, I think the right approach >> would be to scale down the proposal to leave only the focus traversal API >> [0], addressing a single use case of the custom components [1]. >> >> >> >> I am sad to think that there will be no support for custom traversal >> policies in JavaFX in the foreseeable future, as it seems highly unlikely >> that my company will sponsor a major redesign of the focus subsystem. I >> hope there is a non-zero chance of the open source community coming together >> and designing an alternative that satisfies everyone, so maybe not all is >> lost. >> >> >> >> Please let me know what you think of the new proposal. >> >> >> >> Thanks, >> >> -andy >> >> >> >> >> >> [0] >> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/FocusTraversal/FocusTraversal-v2.md >> >> [1] JDK-8091673 Public focus traversal API for use in custom controls >> >> [2] Draft PR https://github.com/openjdk/jfx/pull/1604
