[External] : Re: Proof of concept pull request for Behavior API (PR 1265)

[Andy Goryachev]

Dear John:

I may have come off as very direct

This is not a problem at all; we all are trying to improve the platform as much as we can, using the tools we have.  A mailing list has severe limitations, and time zone difference does not help either.  So please don’t worry about that.  And I hope I did not come out as a stubborn toddler with an NIH syndrome (== not invented here) either.  If I did, please forgive me!

Back to the topic.

You'd create a new class, `MyBehavior`,

By “customizing” I also mean at run time.  Creating new classes wouldn’t work.

coupling

I don’t think it is our choice - it is up to the skin designed.  If they add a node that needs to take input, or if the behavior is drastically different, it is almost impossible to create a common interface.  So skin and behaviors are coupled, besides we have to design for the worst case (of a totally different skin).  The division between S and B comes mostly from the division between V and C in MVC.  From a distance, the user does not see it at all - all they see is a control.

This suggest another metric at judging the usefulness of a design - how easy it is to understand and perform 80% of most common tasks.

There are more interesting ideas at the end of the message I am replying to - fxml, css, global changes - these go far beyond the simple input map improvement.  I did mention this already, but neither open source community, nor my employer might have the resources to make such drastic changes.

So we have to be realistic, I think.  We are travelling to a different planet in a small spaceship and we only have so much material and oxygen to play with.  A simple improvement that helps 80% of use cases might be better than a major redesign (I still think the event proposal involves major redesign).

-andy



From: John Hendrikx <john.hendrikx at gmail.com>
Date: Monday, October 23, 2023 at 14:32
To: Andy Goryachev <andy.goryachev at oracle.com>, openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
Subject: [External] : Re: Proof of concept pull request for Behavior API (PR 1265)

Hi Andy,
On 23/10/2023 20:56, Andy Goryachev wrote:
Dear John:

Thank you for providing the alternative proposal.  One thing I liked is the clarification of the purpose of behavior to translate input events into platform-independent actions.

I noticed you tried to validate the design using Button - a rather simple Control.  I think a better choice would be to pick one of the more complex controls such as TableView or TextArea.
Yeah, I had limited time, but may have some time next weekend to look at a more complicated control.


Another missing piece is probably a set of behavior tests similar to https://bugs.openjdk.org/browse/JDK-8314906 to verify that the behavior does not change from the user perspective.
For now it's a (bare bones) proof of concept, I validated manually that the Buttons still work as before (including things like holding keys down, then messing with it with the mouse) -- there's no reason to assume it wouldn't work, all that was added was a little indirection.


One important problem I tried to solve is managing the key bindings.  I think it’s an important functionality that needs to be provided, evident from the plethora of use cases described earlier in the discussion.  I cannot see from the PR how you propose to address the issue, and the examples you mentioned earlier look to me more complicated than necessary.

The idea would be that you can customize the keybindings by creating a new Behavior.

You'd create a new class, `MyBehavior`, in `install` you'd call `ButtonBehavior.getInstance().install(context)` to install the default keybindings and handlers from whatever base behavior you'd like.  Then you use the `context` to change the bindings you don't like.

The context in this simple proof of concept does not have methods yet to make this easy, but the methods that you provided in `BehaviorBase` could live in the `BehaviorContext` interface in the sample I provided; in other words, a method like `registerKey` could live there; in effect, a method like `registerKey` is the same as installing a KEY_PRESSED handler, but which already does the `== getCode()` check for you (and allows grouping multiple keys in one handler for efficiency).  If `BehaviorContext` becomes too big with all these methods, it could offer a getter to get something where all those methods could live (`InputMap` seems like a decent name for that).

I have hard time understanding where the idea of static or singleton behavior comes from exactly, and what benefits it provides.  The whole design overlooks the fact that behaviors are tightly coupled to skins - to the point where it gets progressively difficult to provide a reasonable skin-to-behavior interface when skins have a different design (in appearance and the kinds and number of nodes involved).

I'm aware, I'm saying that the whole design will benefit immensely from getting rid of this coupling.  You can already see how trivial (and how safe) it is to install a Behavior in the proof of concept; no risk of anything being left behind, full isolation of behavior installed event handlers/listeners is possible, and with the Behavior, BehaviorContext and handlers each handling a particular aspect, it offers a much better separation of concerns.

Perhaps it is impossible to uncouple Skins, but I don't think that is the case, simply because if you build a Skin from scratch with this in mind, you'd be able to do it without the tight coupling; tightly coupling it to skins was just the first iteration that got the job done -- without a clear mental model of what is supposed to be Skin and what is Behavior, things got mixed up and tightly coupled; no doubt that has resulted in a lot of hesitation to make Behaviors public as they're currently more of a Skin implementation detail than a proper separated concern.

With a (possible) definition that Behaviors offer translation to higher level events, Skins can make use of those, but need not use all of them.  This means that a Behavior can also offer translations that may not be useful by current JavaFX skins, but may be useful for custom skins or future adjustments to FX skins. For example, a custom ButtonSkin may not care for "ARM" and "DISARM", but might want to trigger a flashing animation when "FIRE" is triggered.

Adding more events and have them bubble up the hierarchy is, in my opinion, a departure from the current design, possibly a source of regression, impacts performance (however slightly), and is completely unnecessary.  The application developer can easily achieve that with existing mechanisms should such a function is indeed needed.

Perhaps they're not needed, as the functions, if public, are easily called directly as well; they're offered as an alternative to FunctionTags as it is an existing mechanism, and events are quite suited for interpretation and translation to a different meaning (like FunctionTags are offering), or to higher level events with more meaning.  If we can drop FunctionTags, then there'd be no need (yet) for these events.

If we go the event route, the possibilities are left open ended, as the user can decide where and how to deal with them (which need not be at the Control level at all).  It's even possible such higher level events are picked up by a Behavior again (3 "disarms" in a row triggers something new).

It IMHO certainly is worth consideration before dismissing them.

Anyway, I certainly don't agree that they're a regression risk, nor do I think the performance should matter here -- events are already being sent out, a few more is not going to be a problem; events are also in reaction to user actions which are measured in milliseconds.

I also don't see how they're a departure from the current design (and by design I mean current public API's -- not private ones, nor non-finalized ones).


Of course, I can be missing some important points altogether, and/or be completely wrong about what people want (tm).

No, I think key remapping is probably a fair assesment of what people want, although we do disagree a bit at what level it should be offered.  I can imagine a couple of systems:

- Per control mappings

The customization is per control, and all changed mappings need to be (re)applied after constructing the control.

- Global changes of keymappings (ie. something that affects all Buttons with a single call)

The customization happens on a BehaviorFactory level, which if manipulated before any controls are created, would result in all the controls using the changed mappings.

- CSS based (a selector decides which controls gets different mappings)

This could take the form of being able to install a Behavior like you could with Skins (ie. -fx-behavior, -fx-skin), or a completely different form where a CSS property is able to change mappings:

       -fx-key-mappings: addKey(SPACE, fire) removeKey(ENTER)

Indirections could be done the way color indirections are done (some prefixes may be needed):

      .root { fire: BUTTON_FIRE }

- Behavior based (replace a Behavior, but only change the mappings)

Allow a Behavior to be set, and to be easily overriden and its mappings altered.  This would be per control, but differs slightly in that you only need to change one thing (the Behavior) instead of multiple things (each mapping).  It may also open up a path towards doing CSS based installing of behaviors (using -fx-behavior).

Now that Kevin is back from vacation, we could have an internal discussion of the two proposals to determine how we can move forward.

Thanks again for a lively discussion and the PR.

Maybe a bit too lively; I may have come off as very direct (even more so when it is via a text medium) and it took me a while to also see all the issues (and I'm sure I'm still missing alot given limited resources to look into this, and then to provide good feedback/criticism); that was not my intention, I'm just hoping for the best API we can manage, if possible one that looks as if it was part of JavaFX from the start, even if it takes a bit more effort.


Cheers,
-andy



--John



From: openjfx-dev <openjfx-dev-retn at openjdk.org><mailto:openjfx-dev-retn at openjdk.org> on behalf of John Hendrikx <john.hendrikx at gmail.com><mailto:john.hendrikx at gmail.com>
Date: Friday, October 20, 2023 at 14:40
To: openjfx-dev at openjdk.org<mailto:openjfx-dev at openjdk.org> <openjfx-dev at openjdk.org><mailto:openjfx-dev at openjdk.org>
Subject: Proof of concept pull request for Behavior API (PR 1265)
Hi list,

I've made a proof of concept for possibly exposing Behaviors as a first
class API in JavaFX.  It's by no means complete, and there will be some
significant hurdles still no doubt, but this proof of concept does
replace the internal ButtonBehavior completely with a newly implemented
public ButtonBehavior (and it seems to be all working still).

The proof of concept shows the basics behind this proposal, and
introduces a Behavior interface, a BehaviorContext interface, a public
ButtonBehavior and a class with ButtonEvents.

There is also an Alternate ButtonBehavior which (somewhat clumsily)
changes the SPACE key for selecting a button to the "A" key.  I suspect
it shouldn't be too hard to make this a bit more streamlined.

One thing of note that I haven't quite figured out yet; even though the
new ButtonBehavior does not install the Navigation keys, Navigation
still works; this is not because the old behavior is still active behind
the scenes, but just seems to work out of the box.

I'm hoping this will make the idea behind this proposal a bit easier to
grasp.  Note that I'm also mainly trying to make it possible to be able
to remap keys, but I want to make sure that this is done in such a way
that it doesn't block a future open sourcing of a Behavior API.  With
this proposal it's possible however that Behaviors must become public
first though before being able to access the necessary classes to change
input mappings.

The PR can be found here: https://github.com/openjdk/jfx/pull/1265<https://urldefense.com/v3/__https:/github.com/openjdk/jfx/pull/1265__;!!ACWV5N9M2RV99hQ!Mw5TKB4lIWriX9W0DxWvoEysVg7wftVrmL0m_9q_-BwKkvhtFjwsFQOsKfnEdSWiIokRVh3icG1tVJp-orXxdW1WHcID$>

Feel free to comment here or on the PR.  The PR is in draft, so comments
there will not show up on the mailing list.

Note: the test failure is because tests are using reflection to access a
behavior field that is part of skins; this field is no longer present
for ButtonSkin.

--John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231023/78595718/attachment-0001.htm>