On Tue, 19 May 2026 13:28:40 GMT, Marius Hanl <[email protected]> wrote:
>> No, this is **not** a SelectionModel. This is a (missing) part of the basic >> functionality, inability to handle a custom graphic. We typically don't >> notice because it's a corner case, but a corner case where there is no >> workaround using the public API. >> >> I don't quite get the objections here, and I don't particularly like >> exposing half-baked internal implementations in the form of the default >> factory. I also don't completely buy the argument about "set to null if the >> application does not want graphic" because when the application does not >> want a graphic, it does not set a graphic on the tab, right? >> >> What we ended up with after the last commit is too many public APIs for a >> rare corner case, a negative gain. I really don't understand your logic >> here, why make it more complicated? > > This PR is about customizing the menu graphic, right? > So it could very well be a usecase that the menu should have no graphics, but > the tabs should have. > In fact, this is what happens now as well, as the graphic cloning is veryyy > limited to `ImageView` and `Label` only. > If you e.g. use shapes (which I did in the past), no graphic is shown in the > menu. > Now, If I mix Shapes and `ImageView`s, some menu items have a graphic while > others have not - so I might want to extend it or completely remove it. > > Or some Tabs have a graphic (Like the 'Home' Tab) while all others have not - > then it looks very weird that just one tab has a graphic in the menu (and the > space is reserved) while the remaining have not. This is actually something > from my real world experience - therefore something I have interest in. > > My other point is that I really want us to stick to the existing model that > is used in JavaFX. > That is: Null is allowed and will often just kind of 'disable' the > functionality. Which in this case makes sense. > And IMO, we are making things more complicated by handling `null`, > overwriting `invalidated` and even tracking the `lastValue` instead of just > writing `f == null ? null : f.call(t)` and call it a day. fx uses multiple metaphors, there is no single prescribed manner, I think. For example, a null `HBox.alignment` is functionally equivalent to `Pos.TOP_LEFT` (HBox:336) The design principle that I would consider more important is `frequent use cases should be easy, everything else should be possible`. In your case, all you need with this PR is to install a factory that produces a `null` `Node`, and this is not what I would think is the most frequent case, which is just to show the same icons that the application set on tabs. Also, I really dislike the DEFAULT_MENU_GRAPHIC_FACTORY idea - exposing something internal for no purpose whatsoever. Maybe I should just close this PR. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1773#discussion_r3267737376
