[Request for Comments] Behavior / InputMap

Sat Oct 7 17:43:17 UTC 2023

I've read the proposal.

I think it would be good to take a step back first and look at the 
classes and interfaces involved: Control, Behavior, InputMap, Skin -- 
the time is now to take a good look at these before making more public.

They're currently very interwoven which is usually a sign that they are 
not well enough abstracted. They cyclically reference each other 
indicating they have overlapping concerns.  I think this should be 
addressed first before making the existing InputMap and BehaviorBase public.

The main odd thing that jumps out immediately is the fact that Behaviors 
and InputMaps need a reference to their linked Node. This will result in 
chicken-egg style problems where you can't create the Control without an 
InputMap, and you can't create an InputMap without a Control.  It will 
also lead to problems where if a Behavior is replaced that we must take 
care to dispose it, or use only weak references to the Control.  The 
Skin API suffers from this problem, and I think we can do better.

InputMap
========

Looking at InputMap first, I see no real reason why it needs the Node.  
It is only used for installing and removing the event handlers.  This 
action should not be triggered by the InputMap, and instead should be 
inverted so it is triggered by the Control. This can be achieved in 
multiple ways; InputMap could simply produce the required event handlers 
on demand:

      Map<EventType, EventHandler<?>> createEventHandlers();

The Control can then add these to the appropriate handlers, and track 
them for later removal (using a Subscription for example).

Or you could ask the InputMap to install them by providing the Node (the 
idea here being that Node is not stored, but provided as context):

      Subscription installEventHandlers(Node node);

Note that the above returns a Subscription, which can be used to remove 
the event handlers again without requiring an interaction with the InputMap.

The removal should be fairly trivial, and will lead to a better 
encapsulated and easier to reason about InputMap class that does not 
suffer from the chicken egg problem, and is not generically typed.

That brings us to InputMap's being mutable.  I would first spend some 
serious effort to see if this is strictly needed, and what benefits that 
would provide, as currently what I see is that InputMaps are mainly 
mutable in order to add mappings during construction (something a 
Contructor or Builder can do). Immutable InputMaps would simplify things 
enormously, and would prevent mappings being modified (by a Skin or 
Behavior) that must be undone when the Skin or Behavior changes.  This 
would also mean there is no need to have multiple input maps in memory.  
Cells have behaviors, and you can easily have 1000's on screen, 
requiring 1000's of Behavior and InputMap copies currently.

If they must be mustable however, then controls must listen for changes; 
this will inevitably mean that an InputMap refers each Control that 
listens for changes.  For a shared global InputMap this can mean that 
such Controls won't be GC'd, so we need to apply one of the usual 
strategies here to prevent this problem:

1. Use a weak listener (relying on GC)
2. Use a conditional listener (deterministic)

If InputMaps must be mutable, then the second one has by far the 
preference, and can be done by only listening for InputMap changes when 
the control is showing.  The best way to achieve this is for Controls to 
delay registering the InputMap's event handlers until they're about to 
become shown, and remove the event handlers when they're about to be 
unshown.  Note that this is not the same as visible/invisible and so 
won't lead to registration/unregistration upon toggling visibility.

Behavior
=======

First, I think there should be a real Behavior interface.  A required 
abtract base class will just lead to the tempatation to use JavaFX 
internals when things get inconvenient for internal Controls (using the 
accessor pattern), and will result in users not being able to replicate 
all the behaviors that FX controls can exhibit (as they won't have 
access to the same internals).

BehaviorBase immediately shows the same problem as InputMap. Even though 
you'd expect that a Behavior should be sharable between controls (it is 
after all the case that for most controls of a certain type all their 
behaviors will be the same) it has elected to explicitely couple itself 
to a specific control instance.  The base class itself only does this 
because InputMap needs the Node, however, subclasses are using this 
judiciously everywhere to access the Node linked to a Behavior.

As Behaviors respond to events however, there is I think no real need to 
have a direct reference to the Node -- the Event provides this already; 
it just needs to be extracted, or perhaps in some cases, this 
information needs to be added first as it is pertinent to the event.

Looking at various uses of `getNode` (used in about 200-300 places) I 
haven't found one yet that isn't responding to an event.  This makes 
sense, as that's probably the primary way a Behavior does its thing, it 
install event handlers and responds to them.

So I think that with a bit of refactoring, Behaviors can be made to not 
require the Node.  This will then allow Behaviors to be installed with a 
trivial setter, and would not need the install/dispose mechanism that 
Skins use.

TLDR;

I think that we should really be seeing if it is possible to remove the 
dependency on Controls before making these API's public.  This would 
remove the generic parameter from the signatures of both InputMap and 
Behavior(Base), and remove the chicken-egg dependency they currently 
have with Control.  It will simplify the design, and probably make a lot 
of classes and code a lot easier to reason about.

--John

On 30/09/2023 00:44, Andy Goryachev wrote:
>
> Dear fellow JavaFX developers:
>
> For some time now, we’ve been working to identify missing features in 
> JavaFX that hinder application development.  We’ve been working on 
> adding some of the missing features (for which we’ll have a separate 
> announcement), but I feel that engaging wider community is a rather 
> important part of the process.
>
> I would like to share with you one such missing feature - ability to 
> extend behavior of the existing components (and make the task of 
> creating new components easier) by adding a public InputMap and 
> BehaviorBase.
>
> Please find the actual proposal here
>
> https://gist.github.com/andy-goryachev-oracle/294d8e4b3094fe16f8d55f6dd8b21c09
>
> We are very much interested in your feedback.  Thank you in advance.
>
> -andy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231007/bf42d25f/attachment-0001.htm>