On Mon, 18 May 2026 21:12:25 GMT, Marius Hanl <[email protected]> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 26 additional >> commits since the last revision: >> >> - factory >> - Merge branch 'master' into 8353599.menu.factory >> - review comments >> - Merge branch 'master' into 8353599.menu.factory >> - Merge branch 'master' into 8353599.menu.factory >> - review comments >> - Merge branch 'master' into 8353599.menu.factory >> - Merge branch 'master' into 8353599.menu.factory >> - Merge branch 'master' into 8353599.menu.factory >> - 2026 >> - ... and 16 more: https://git.openjdk.org/jfx/compare/ced9934b...354a515b > > 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? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1773#discussion_r3262288713
