Dear John: Based on what I understand about your proposal now, I think it’s a variant of the same thing, not fully developed and in some places based on the incorrect assumptions (S/B separation, stateless B). But you know I am biased :-)
A more detailed proposal - up to you; there are literally thousands of bugs in FX that I’d rather focus on instead, but it’s an open source and nobody can tell you what to do :-) May be a more detailed proposal at least outlining public APIs might help. Looking at your Carousel TreeViewSkin demo (nice work, by the way!) I fully agree - there is no way we can reasonably create a base skin for that. It is a new skin, or maybe even a new control altogether. The separation between skins and behaviors are fuzzy, but we do have an established and well know pattern: MVC. Behavior is the controller in this paradigm - a stateful logic which translates input events into changes in model. You are proposing for B to translate the input events to another set of events which I think is unnecessary. “Everything should be made as simple as possible, but not simpler”, to paraphrase Herr Einstein. May be I don’t see a clear use case (for new events)? A behavior listens for events (coming from control or elements of skin), maintains its state, translates these events into direct actions on the model or the control (control being a façade). I think there is no need for an extra layer of events, especially if they bubble up. Most use case I think are limited to developers changing a minor aspect of the skin/behavior - adding a corner node between horizontal and vertical scroll bar for example, or modifying the way currently selected item is highlighted vs. the mouse hover highlight, remap a key binding, or something like that. Go beyond that - it’s a new thing altogether, as you rightfully said earlier. Also, when looking at sufficiently complex control such as (Rich)TextArea or TreeTableView - I don’t think the behavior can be composited from a bunch of simple things - there is too much interaction. So B ends up being a complex, stateful beast that tightly coupled with the particular implementation of the skin (so in some sense you are right, saying we have only skins). Just like ComponentUI in swing. It simply cannot be an interface or a singleton. I am sorry, John, for my negative reaction to your proposal. I am very grateful for the discussion, and I think we should discuss/develop some ideas in it further - like prioritization of event handlers. What do you think we ought to do to move forward? Thank you -andy From: John Hendrikx <john.hendr...@gmail.com> Date: Tuesday, October 17, 2023 at 15:04 To: Andy Goryachev <andy.goryac...@oracle.com>, openjfx-dev@openjdk.org <openjfx-dev@openjdk.org> Subject: Re: [External] : Re: [Request for Comments] Behavior / InputMap 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<https://urldefense.com/v3/__https:/www.youtube.com/watch?v=8Mb15bOwIyE__;!!ACWV5N9M2RV99hQ!OI9hc6cUqJ-poC0z2krLl8yLq80ITMZf9oG0RF-IK9xfrErmMWvPWqrC9ZXvNvLIbxBHJT9b_nSoBSe-eWYdyUbCCOi6$>) -- 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
