<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>I've read the proposal.</p>
<p>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.<br>
</p>
<p>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.</p>
<p>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.</p>
<p>InputMap<br>
========<br>
</p>
<p>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:</p>
<p> Map<EventType, EventHandler<?>>
createEventHandlers();</p>
<p>The Control can then add these to the appropriate handlers, and
track them for later removal (using a Subscription for example).<br>
</p>
<p>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):</p>
<p> Subscription installEventHandlers(Node node);</p>
<p>Note that the above returns a Subscription, which can be used to
remove the event handlers again without requiring an interaction
with the InputMap.</p>
<p>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.</p>
<p>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.</p>
<p>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:</p>
<p>1. Use a weak listener (relying on GC)<br>
2. Use a conditional listener (deterministic)<br>
</p>
<p>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.</p>
Behavior<br>
=======
<p>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).</p>
<p>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.</p>
<p>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.</p>
<p>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.<br>
</p>
<p>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.</p>
<p>TLDR;<br>
</p>
<p>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.</p>
<p>--John</p>
<br>
<div class="moz-cite-prefix">On 30/09/2023 00:44, Andy Goryachev
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:DM5PR1001MB217215F738164DBF5FC16EF0E5C0A@DM5PR1001MB2172.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="Generator" content="Microsoft Word 15 (filtered
medium)">
<style>@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face
{font-family:"Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}@font-face
{font-family:"Iosevka Fixed SS16";
panose-1:2 0 5 9 3 0 0 0 0 4;}@font-face
{font-family:"Times New Roman \(Body CS\)";
panose-1:2 11 6 4 2 2 2 2 2 4;}@font-face
{font-family:"\@Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
mso-ligatures:standardcontextual;}a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Iosevka Fixed SS16";
color:windowtext;}span.apple-converted-space
{mso-style-name:apple-converted-space;}.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}div.WordSection1
{page:WordSection1;}</style>
<div class="WordSection1">
<p class="MsoNormal"><span style="font-family:"Iosevka
Fixed SS16";color:#212121">Dear fellow JavaFX
developers:</span><span style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121"> </span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121">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.</span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121"> </span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121">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. <span class="apple-converted-space"> </span></span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121"> </span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121">Please find the actual proposal
here</span><span style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121"><a
href="https://gist.github.com/andy-goryachev-oracle/294d8e4b3094fe16f8d55f6dd8b21c09"
moz-do-not-send="true" class="moz-txt-link-freetext">https://gist.github.com/andy-goryachev-oracle/294d8e4b3094fe16f8d55f6dd8b21c09</a><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka
Fixed SS16";color:#212121"> </span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121">We are very much interested in
your feedback. Thank you in advance.</span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121"> </span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal" style="caret-color: rgb(33, 33,
33);font-variant-caps: normal;orphans:
auto;text-align:start;widows: auto;-webkit-text-stroke-width:
0px;word-spacing:0px">
<span style="font-family:"Iosevka Fixed
SS16";color:#212121">-andy</span><span
style="color:#212121"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-family:"Iosevka
Fixed SS16""><o:p> </o:p></span></p>
</div>
</blockquote>
</body>
</html>