Public Behavior API proposal

[John Hendrikx]

On 13/11/2023 07:14, Michael Strauß wrote:
> Hi John,
>
> I think this is an excellent summary of all the discussions so far,
> and the best proposal I've seen for a comprehensive control
> architecture capable of addressing many shortcomings of the existing
> controls.
>
> Here are some thoughts:
>
> 1. Semantic events == Interaction API
> Previously, skins were a black box with close to no extensibility.
> Re-implementing skins is often a no-starter because you'll instantly
> lose all of the intricate behaviors that are invisibly built into the
> skins. Semantic events are a game changer, as they clearly define the
> API of a skin, i.e. the kind of interactions that a well-behaved skin
> will support. A custom skin doesn't need to recreate the entirety of
> its control behavior from scratch, it should be enough to implement
> the interaction API defined by its control's semantic events. If you
> consider ActionEvent (which *is* a semantic event), it seems obvious
> to me that more semantic events are a natural evolution of JavaFX.
>
> 2. Clear separation of concerns
> All parts in the control architecture now have a clearly defined
> purpose. Some of the restrictions that come with each of the parts
> could be enforced by API, others might need to be explained in
> documentation. The roles and responsibilities of each part of the
> control architecture are easy to understand with a small set of rules
> (what's allowed and what's not allowed).
>
> 3. Behaviors
> I'm not sold on the Behavior design as proposed in the GitHub
> document, specifically the idea of BehaviorContext and using
> composition to extend existing behaviors. The API feels rather clunky,
> and maybe unnecessary. Perhaps mirroring the Skin design (i.e. an
> interface that gets a Control reference) can narrow down the API
> surface.

The Skin design is not very well done in my opinion, and I think many 
will agree, as it was recently refactored. Mirroring it would be 
repeating the same mistake.

A Skin is used in a few ways currently:

- As a thing to set on an unspecified/any Control (not good to construct 
this with a Control)
- As a thing that contains state for a *specific* Control (perfectly 
fine to construct this with a Control)
- As a thing that tracks modifications it did to Control for later 
uninstalling (should you rely on Skin for this?)

Skins are currently like a Factory class that contains the state of its 
Product; for each new Product you need to construct a new Factory.  The 
Factory can be used only once, and can't be reused. The solution is of 
course to put the state in the Product.  The factory is now reusable.

Same with Skins. Remove the Control reference from its constructor, pass 
the Control reference during installation and create a "Product" which 
contains the state.  Skins are now reusable with minimal effort.  No 
need to check if the Skin that is installed is referring to the correct 
Control, or if that Skin was "used" before.  Perhaps what we call Skin 
should be SkinFactory, but as the stateful part (which would be called 
Skin) will be hidden anyway, the name Skin remains perfectly available. 
Naming them "Skin" and "InstalledSkinState" or even "Skin.State" 
(they're non public anyway) would be accurate as well.

So let's not repeat this with Behaviors.

The BehaviorContext is there for multiple reasons; it could just a 
Control reference, but I find that its better to enforce what an 
implementation is allowed to do then to only put some rules in the 
documentation. Rules can be broken (just see the implementation of 
com.sun.javafx.css.BitSet which broke almost every rule for Sets 
imaginable before it was fixed).  By enforcing the rules we give 
guidance to how a Behavior should be constructed.  BehaviorContext is 
nothing more than a Control reference which has some methods crossed out 
as "not allowed" -- avoid giving functions more than they need (ie. 
don't give a "sendMail" function an Employee object when only its 
Address would do), it makes things far easier to reason about (a 
Behavior couldn't have done X, as we didn't expose X).

BehaviorContext also serves another purpose, which I think is incredibly 
valuable: it allows the Control to track what changes were made by the 
Behavior when it was installed, and also gives it control of HOW they 
are installed (priority wise for example). If you give it a direct 
Control reference, you have to rely on the Behavior later to do this 
correctly, and also (with a `dispose` method) to "undo" its actions, 
possibly leaving behind things that later lead to weird bugs in a newly 
installed unrelated behavior -- lots and lots of effort was put into 
Skins to clean up after themselves properly, leading to at least two or 
three different "tracking" systems, many many bugs and many discussions 
-- we should not leave this up to the implementation, instead enforce 
it, and debug it once. When you are able to track all the changes, you 
can uninstall any Behavior, no matter how unruly, without its 
cooperation. Not only does this keep the Control in control of its 
internal structures, it allows the control to install such event 
handlers differently (with a lower priority for example), and on top of 
that means that **all** Behaviors can be lighter weight as they don't 
need to track what they installed themselves (this is solved one time in 
one location, instead of in every behavior).

An API like this addresses all the concerns the different actors have:

- The CSS engine and Users just want to select and install a behavior 
without having to create a new one each time (possibly pre-attuned to a 
specific control) -- solved by making Behaviors static.
- The Control wants to ensure the User has the last say on anything, 
which it can't guarantee if Behaviors can do whatever they want
- A Behavior implementation wants to follow the rules set out for them 
(enforced with BehaviorContext)

> Note that, like with skins, this doesn't mean that a behavior
> should do things it isn't supposed to be doing. It should be easy to

So let's enforce it, instead of relying on documentation as guard rails.

> override aspects of behavior (as you put it, the "illusion" that user
> code is the only code), and it is not entirely clear if this requires
> an enhancement of the JavaFX event system.
Maybe it can be done without an enhancement, if the Control can separate 
event handlers installed by Behaviors.
> 4. InputMap proposal
> All things considered, I don't think that the current InputMap
> proposal carries its weight in terms of complexity and API surface.
> Doing InputMap first and in its current state might foreclose on our
> ability to revamp the control architecture to be more flexible and
> extensible. I do think that InputMap would benefit from having a new
> control architecture in place, and being more tailored towards
> controls that really need it (as discussed before, not all controls
> will benefit from InputMap).
I have the same worries.  I think the Behavior design must come first, 
only then can we know for sure how exposing a concept like InputMap will 
fit.  The main culprit being how to represent high level actions; if 
they're FunctionTags then it can be hard for users to intercept them or 
for Skins to generate them.  Perhaps leaving FunctionTags out for now 
would be viable for a first implementation.
> 5. Migration compatibility
> In order to have any chance of gaining traction, we need a story how
> this proposal can support gradual migration without breaking
> compatibility with existing controls.

I think it should be possible to do this one control at a time.

--John

>
>
> On Mon, Nov 13, 2023 at 1:12 AM John Hendrikx <john.hendrikx at gmail.com> wrote:
>> Hi everyone, and specifically Andy and Michael,
>>
>> I'm working on updating the Behavior API proposal, and I've been
>> thinking about the semantic events a lot.  I would really like to hear
>> what you think, and how it matches with your ideas.
>>
>> Quick recap, Semantic Events are high level events (like the ActionEvent
>> from Button) that can be triggered by a combination of low level events.
>> They represent an action to be achieved, but are not tied to any
>> specific means of triggering it.  For example, the ActionEvent can be
>> triggered with the mouse, or with the keyboard; it is irrelevant which
>> one it was.  Semantic events can be generated by Skins (as a result of
>> interactions with the Skin's managed children), by Controls (see below)
>> and users directly. You can compare these with Andy's FunctionTags or
>> Actions from various systems.
>>
>> Let me describe exactly each part's role as I see it currently:
>>
>> # Controls
>>
>> Controls define semantic events, provides infrastructure for handling
>> events that is separated from internal needs (user comes first). User
>> installed event handlers always have priority to make the user feel in
>> control. The Control also provides for another new infrastructure, the
>> managing of key mappings.  The mapping system can respond directly to
>> Key events (after the user had their chance) to generate a semantic
>> event.  This means that both Control and Skin are allowed to generate
>> semantic events, although for Control this is strictly limited to the
>> mapping system.  The key mappings are only overrideable, and their base
>> configuration is provided by whatever Behavior is installed.  Exchanging
>> the Behavior does not undo user key mapping overrides, but instead
>> provides a new base line upon which the overrides are applied.  So if a
>> Behavior provides a mapping for SPACE, and the user removed it,
>> installing a different behavior that also provides a mapping for SPACE
>> will still see that mapping as removed.  If a behavior doesn't define
>> SPACE, and the user removed it, then nothing special happens (but it is
>> remembered).
>>
>> - Controls refer to a Skin and Behavior
>> - Controls define semantic events
>> - Controls can generate semantic events (via mappings)
>> - Controls never modify their own (user writable) state on their own
>> accord (the user is in full control)
>> - Controls provide an override based key mapping system
>>
>> # Skins
>>
>> Skins provide the visuals, and although they get a Control reference,
>> they are restricted to only adding property listeners (not event
>> handlers) and modifying the children list (which is read only for users
>> as Control extends from Region).  This keeps the user fully in control
>> when it comes to any writable properties and events on Control.  Most
>> Skins already do this as I think it was an unwritten rule from the
>> beginning.  Skins then install event handlers on their children (but
>> never the top level Control) where translation takes place to semantic
>> events.  Skins have no reference to the Behavior to ensure that all
>> communication has to go through (interceptable) semantic events.  Not
>> all events a Skin receives must be translated; if some events only
>> result in the Skin's internal state changing, and does not need to be
>> reflected in the Control's state then Skins can handle these directly
>> without going through a Behavior.  Examples might be the position of the
>> caret, or the exact scroll location of a View, if such things aren't
>> part of the Control state.
>>
>> - Skins refer to a Control (legacy inheritance) but are limited in their
>> allowed interactions (unwritten rule since the beginning)
>>     - Better would be to provide skins with only a Context object that
>> only allows installing of listeners to ensure they can't do nasty things
>> (and to track all changes a Skin did for when it is replaced, similar
>> idea to BehaviorContext)
>> - Skins interprete normal events of their CHILDREN only (after the user
>> did not consume them), and either:
>>      - Translates them to semantic events (targetted at the top level
>> Control)
>>      - Acts upon them directly if only Skin internal state is involved
>> - Skins never act upon semantic events
>>
>> # Behaviors
>>
>> Behaviors provide a standard set of key mappings, and provide standard
>> ways of dealing with semantic events.  Installing a new Behavior on a
>> control can make it "feel" completely different, from how it reacts to
>> keys, and which keys, how it deals with the high level semantic events,
>> as well as how it handles mouse interactions.  Behaviors can act upon
>> both normal and semantic events, but don't generate any events
>> themselves.  Again, they only act upon events after the user had a
>> chance to act upon them first. A behavior is free to act upon Key events
>> directly (for things too complicated for a simple key mapping), but it
>> would be better to indirect them as much as possible via a semantic
>> event that is triggered by a key mapping in the Control.  Mouse events
>> are more complicated and don't need to be indirected to be handled (not
>> 100% sure here yet).   When receiving a semantic event that the user
>> didn't care about, the Behavior consumes it and does its thing by
>> calling methods on the Control and modifying control state.
>>
>> - Behaviors refer to nothing
>>      - Control reference is received via event handler and listeners only
>> - Behaviors define base key mappings
>>      - Controls refer to these, after first checking for any user overrides
>>      - Base key mappings can be global and immutable, user overrides
>> (maintained in Control) are mutable
>> - Behaviors never generate events
>> - Behaviors can act upon events of any type (if unconsumed by the user)
>> that are targetted at the control (enforced as that's the only place
>> they can install handlers)
>> - Behaviors are allowed to modify control state and their own state
>>
>> # Interaction with Andy's proposal
>>
>> I think the above works pretty nicely together with Andy's proposal.  By
>> moving the responsibility managing the key mappings to Control, but
>> leaving the responsibility of defining the mappings with Behaviors, I
>> see a nice path forward to opening up a key mapping system for simple
>> overrides, as well as having a public Behavior API for more advanced
>> behaviorial modifications.
>>
>> Notice that I didn't provide for a FunctionTag remapping system; as
>> these would be semantic events in this proposal, they can be
>> filtered/handled before the Behavior gets them.  So to change a
>> function, just consume it and fire a different one.  To completely block
>> it, just consume it.  To replace a single function with two functions,
>> consume it and fire two new ones, etc.  So to globally swap the
>> increment/decrement functions of all Spinners, at the Scene level you
>> could have a handler do this.
>>
>> To also answer Andy's 10 questions:
>>
>> Q1. Changing an existing key binding from one key combination to another.
>>
>> -> Control provides a small API to override base mappings (remap)
>>
>> Q2. Remapping an existing key binding to a different function.
>>
>> -> Control provides a small API to override base mappings, including to
>> which semantic event they translate
>>
>> Q3. Unmapping an existing key binding.
>>
>> -> Control provides a small API to override base mappings (disable)
>>
>> Q4. Adding a new key binding mapped to a new function.
>>
>> -> Still debatable if this should be provided, but I don't see much
>> blockers for this; someone will have to interpret the new function
>> though; this could be a user event handler that knows the event (which
>> can be a custom one) or a customized Behavior.
>>
>> Q5. (Q1...Q4) scenarios, at run time.
>>
>> -> All possible at runtime.  With a "-fx-behavior" CSS feature, this
>> could also be provided via CSS selectors, allowing far reaching changes
>> without having to modify each control individually.
>>
>> Q6. How the set behavior handles a change from the default skin to a
>> custom skin with some visual elements that expects input removed, and
>> some added.
>>
>> -> Behaviors act only upon events targetted at the Control. Skins that
>> don't provide some events means they will just not be picked up by
>> Behaviors.  Skins that provide unknown semantic events require a
>> corresponding Behavior upgrade.  Skins actions that don't require
>> Control state changes (only Skin state changes) can ignore this system
>> altogether.
>>
>> Q7. Once the key binding has been modified, is it possible to invoke the
>> default functionality?
>>
>> -> Yes, just fire the appropriate semantic event at the Control
>>
>> Q8. How are the platform-specific key bindings created?
>>
>> -> Debatable if this is needed, but probably something similar to your
>> proposal will be possible; Platforms don't change at runtime, so why
>> they are even added as bindings (instead of just skip adding them if not
>> on the right platform) is a mystery to me.  A simple tool (perhaps on
>> the Platform class) to check the platform should be sufficient; no need
>> to interweave this with key mappings themselves.
>>
>> Q9. How are the skin-specific (see Q6) handlers removed when changing
>> the skins?
>>
>> -> Skins clean up after themselves, and they're not allowed to install
>> handlers on the control (only on their children)
>>
>> Q10. When a key press happens, does it cause a linear search through
>> listeners or just a map lookup?
>>
>> -> No, Controls have freedom to optimize how they do this; Behaviors
>> provide base mappings in some kind of Map form, or have an API to
>> quickly look up a base mapping (probably the latter to encapsulate it
>> better).
>>
>> Thanks for reading,
>>
>> --John
>>