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
