On 06/08/2025 18:06, Martin Fox wrote: >> On Aug 5, 2025, at 4:07 PM, John Hendrikx <john.hendr...@gmail.com> wrote: >> >> >> On 05/08/2025 22:01, Martin Fox wrote: >>> I have some questions related to the spec. I think some of this has been >>> addressed in the PR but not in the documentation. >>> >>> - This spec goes through some trouble to ensure that the target of a >>> KeyEvent is the ComboBox/Spinner and not its TextField. >> This is not new in this spec though. KeyEvents are delivered to the >> focused control. The focused node has always been the Control, and not >> any part of its Skin. > The TextField of a ComboBox or Spinner is not just an artifact of its skin. > They are both explicitly composite controls that contains a TextField inside. > It’s part of the API. And if you were to install some filters on the Scene to > track Key and InputMethod events you could easily conclude that that > TextField is the focus owner (it walks like the focus owner, it quacks like > the focus owner).
I disagree. A ComboBox and Spinner are primitives. Their implementation via replacable Skins may or may not use a TextField, that's completely up to the Skin. Some details do leak out, but those mainly used to enable CSS styling. CSS is built for this and will leniently ignore untargetable styles. >From the perspective of the user, the Control is a leaf Node, the only target for focus and should (IMHO) also be the only target for mouse clicks. Control goes through great lengths to construct a black box here, integrating CSS properties supplied by the Skin and forwarding methods like `compute` and `layoutChildren` to the Skin. Changing this now means Controls are just Regions and there is no point in even having the class. The fact that some of this currently is leaky (I'm looking at listeners and event handlers added by skins and behaviors that intermix with user handlers) does not take away that it was always intended to be a skinnable black box, where no assumptions can and should be made as to what the Skin supplies or how it is implemented. > I believe the implementation of Spinner predates focusWithin and > focusVisible. I think the natural way to drive the focus indicators these > days would be to use focused for the TextEdit and focus-within for the > Spinner. In any case I think it’s a good idea to somehow separate these out > and not force all of these flags to be set in unison across the entire > delegate chain. I think that Controls fill two roles, as a leaf node for users and as an extension point (a Parent) for Skins. This has become intermixed in FX by reusing existing API's because they were conveniently available, yet not quite suited for purpose. - Controls are or should be leaf nodes for focus, events and for users querying children; users should never be concerned with the implementation provided by the Skin. Events should be forwarded to a Control's children decided by the Control (or actually its Behavior). This can be also be done without having that Event traverse down from Scene, by building a custom event dispatch chain that only travels from Control to one of its descendants. The fact that some behaviors/skins currently don't realize that this is possible is very telling. Michael's solution just makes this re-usable, but Behaviors can easily install handlers on Control, and do their own event forwarding without going down from Scene (they still "duplicate" the event, but no Node ever sees the same event twice). - Skins supply additional controls (optionally) that are added to the children list of Control -- this was a mistake, they should have been added to a separate list only available for CSS styling and SceneGraph rendering -- the children of a control supplied by the Skin are not even there yet until the first CSS pass completes... - Skins re-use listener and event handler endpoints on the Control, leaving users baffled by events sometimes being consumed before they see them (if they added their handler late) or after they see them (if they were the first to add their handler, or if the Skin was replaced causing a re-ordering -- something very interesting to deal with); for listeners this is relatively safe, as they don't have a consumption mechanism, but not for events -- another mistake; skin event handlers should have been part of a separate list so there can never be any intermixing of user and skin event handlers What we're currently seeing is that there are some leaks in the black box that is supposed to be Control, and what you seem to be proposing is that instead of plugging those leaks we create more. > If we were to make the Spinner’s TextEdit the focus owner input methods would > work, the focused and focus-within pseudo-classes would sort themselves out > in a natural way, and the double firing of KeyEvents would go away. The > downside is that you would create an information deficit during event > processing; the event target would be the TextEdit and not the logical > control. But that’s an existing problem that also affects mouse click events. > Perhaps we should focus on closing that information gap instead. > > Martin > >
