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-r...@openjdk.org> on behalf of John
Hendrikx <john.hendr...@gmail.com>
*Date: *Friday, October 20, 2023 at 14:40
*To: *openjfx-dev@openjdk.org <openjfx-dev@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
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
