[External] : Re: [Request for Comments] Behavior / InputMap

[John Hendrikx]

Hi Andy,

On 17/10/2023 20:07, Andy Goryachev wrote:
>
> Dear John:
>
> It looks like we have different views on the subject, so perhaps we 
> should invite other people to weigh in.
>
I would be interested to hear from others on this subject as well.

I feel however that I may need to make a more formal proposal to show 
more clearly what is possible.  I'd still be interested in hearing your 
views on the alternative proposal after I addressed your initial 
questions.  Do you need more information?

> My goal is to move forward adding missing features without making any 
> drastic changes, and avoid regressions as much as possible.  We also 
> have to be mindful of the fact that we are dealing with constrained 
> resources, so any major development is very likely out of question. 
> That’s why
>
I don't think my proposal is that drastic, nor do I think it will be the 
cause of any major regressions.  Nor do I agree that my alternative is 
that much harder to implement.  It may require a bit more work upfront, 
but it also offers more in the end.  It still can be implemented one 
control at a time.

> My problem here is that by making this class public (which IMHO is not 
> a good design) will lock us out of improving this later.
>
> If nobody uses FX there will be no “later”.  If customers’ code breaks 
> too often they will switch to some [unnamed] alternative.
>
That's conditional on if we're breaking code, who said anything about 
that?  Are you saying we can't change internals too much?

> Nothing Behavior related should be made public without a full design 
> of how Behaviors should work, what their responsibilities are, how 
> they interact with Skins (if at all, IMHO they shouldn't), etc.  Once 
> a design is known and agreed upon, then we can work on step-by-step 
> improvements by making parts public, and leaving parts hidden.
>
> We kind of know what behaviors do - they handle user input, modifying 
> the appearance or internal state of the control.
>
> I just want to mention again that behaviors must interact with skins - 
> not all the behaviors, but some.  For example, TextArea 
> “MOVE_LINE_END” needs to know how the text is laid out in order to 
> determine the target caret position, this function cannot be 
> implemented without the skin and the laid out text.  I think we need 
> to take into account this constraint.
>
This is a technical issue that can be resolved.  Tying Behaviors and 
Skins together just means we only have one thing: Skins.  In that case, 
why pretend there even are behaviors?  If Skins need to provide 
functionality, this can be achieved differently.  TextArea could provide 
overridable hooks for this, Skins could install an event handler for 
TextAreaEvent.MOVE_LINE_END; just the fact that the TextArea skin is 
doing it this way is a red flag that clearly shows this class may need 
some adjustments as it was clearly not well separated -- it's normal for 
such problems to appear when trying to introduce something new; you 
don't work around them, you solve them.

> Another point is that behavior is sometimes stateful, so let’s take 
> this into account as well.
>
I've taken it into account now, and state can be associated with 
behaviors easily enough, without having a behavior be both the state 
class and the factory.  It's a minor separation, but makes it much 
easier to reason about.
>
> I think BehaviorBase is just a convenient way to access the new input 
> map (the subject of the new proposal). If someone does not want or 
> does not need to use the input map - fine, nothing in the proposed 
> design requires them to use it.  I don’t think it should be an 
> interface either - the methods in BehaviorBase are protected because 
> they are offered for subclasses only, and making BB an interface will 
> turn these public which isn’t right.
>
I have a better solution for this, allowing Behavior to be an interface, 
with easy subclassing and composition options to manipulate the 
contained inputmap (without actually needing to expose an inputmap).
>
> I do acknowledge that the issue of ordering of calls to added handlers 
> might be undefined/unexpected, especially after a skin change.  This 
> is the existing condition, and we probably should try to solve it in a 
> separate thread.  I think that for skins the expectation is that the 
> handlers added by the user are orthogonal to those added by the 
> skin/behavior, and if they are not - use event filter, or let’s talk 
> about adding prioritization in a separate discussion.
>
I always expected such a fix to be a separate change.  It could even be 
delayed somewhat, but not forever.  As the current implementation does 
not specify any ordering, there is sufficient freedom to make minor 
adjustments here.  Again, event filters are not intended and 
insufficient for this purpose.
>
> I also acknowledge that the input map proposal does not address the 
> issue of creating alternative custom skins and extending behaviors 
> beyond modifying the key mappings and some limited manipulations of 
> handlers.  But, in my opinion, it adds substantial value to the 
> application developers without upsetting the cart too much.
>
Skins are sufficiently easy as it is, and I don't see it as a goal to do 
anything here, as long as we keep them separated not much thought needs 
to go here; designing Skins for possible extension (with well specified 
protected methods that can't be changed anymore as they're API) is a 
near impossibility so I don't see that happening ever.  FX Skins are 
just too complex for that (they're not really what people view 
traditionally as Skins, just some graphics that can be replaced).

Skinning will IMHO remain a business of writing a complete new skin, as 
the options are endless and any customization of an existing Skin is 
bound to almost always run into a place where insufficient customization 
options will force the creation of a new Skin -- you simply can't design 
for the unknowable.  For example, I have Skins for ListView that turn it 
into a column based view, and one that turns it into a fully 3d animated 
image carousel (https://www.youtube.com/watch?v=8Mb15bOwIyE) -- there's 
just no way that could be achieved "customizing" ListViewSkin.

Behaviors are however much more limited, and not nearly as complex; they 
can be and should be extendible in the future.

> They say “better is the enemy of good”, which is apt in our case, 
> though I hope we can somehow agree on a solution in a reasonable time.
>
That's the rule for code that can be changed in the future :) For API's 
the rule is: if you aren't sure, then don't.

--John

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