On Mon, 18 May 2026 21:29:06 GMT, Andy Goryachev <[email protected]> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
>> line 303:
>>
>>> 301: }
>>> 302: set(lastValue);
>>> 303: throw new NullPointerException("cannot set
>>> factory to null");
>>
>> I think we should allow `null` and just return a `null` graphic then. No
>> need to do any handling here, less logic.
>> Just:
>>
>> private Node createGraphic(Tab t) {
>> Callback<Tab, Node> f = getMenuGraphicFactory();
>> if (f == null) {
>> return null;
>> }
>> return f.call(t);
>> }
>>
>>
>> And this is also aligned with other components in JavaFX, like e.g. `null`
>> SelectionModel.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1773#discussion_r3266657107
