On Wed, 13 May 2026 23:17:16 GMT, Andy Goryachev <[email protected]> wrote:
>> Tries to address the mystery of missing graphic in the TabPane overflow menu. >> >> ### Summary of Changes >> >> - minor `TabPaneSkin` constructor javadoc clarification >> - added the property >> - changed popup menu to be created on demand >> - removing adding the popup reference to the `TabHeaderSkin` properties (I >> think it was done for testing purposes, I could not find any references to >> it in the code) >> >> For a quick tester, use >> https://bugs.openjdk.org/secure/attachment/114240/TabPaneGraphicFactoryExample.java >> >> # Overflow Menu Graphic Property in the TabPaneSkin >> >> Andy Goryachev >> >> <[email protected]> >> >> >> ## Summary >> >> Introduce a `menuGraphicFactory` property in the `TabPaneSkin` class >> eliminates the current limitation of this skin >> in supporting menu item graphics other than an `ImageView` or `Label` with >> an `ImageView` graphic. >> >> >> >> ## Goals >> >> The goals of this proposal are: >> >> - to allow the application developers to customize the overflow menu items' >> graphic >> - retain the backward compatibility with the existing application code >> - clarify the behavior of the skin when the property is null (i.e. the >> current behavior) >> >> >> >> ## Non-Goals >> >> The following are not the goals of this proposal: >> >> - disable the overflow menu >> - configure overflow menu graphic property via CSS >> - add this property to the `TabPane` control itself >> >> >> >> ## Motivation >> >> The existing `TabPaneSkin` does not allow the overflow menu to show graphic >> other than >> an `ImageView` or `Label` with an `ImageView`. >> >> This limitation makes it impossible for the application developer to use >> other graphic Nodes, >> such as `Path` or `Canvas`, or in fact any other types. The situation >> becomes even more egregious >> when the tabs in the `TabPane` have no text. >> >> Example: >> >> >> public class TabPaneGraphicFactoryExample { >> public void example() { >> Tab tab1 = new Tab("Tab1"); >> tab1.setGraphic(createGraphic(tab1)); >> >> Tab tab2 = new Tab("Tab2"); >> tab2.setGraphic(createGraphic(tab2)); >> >> TabPane tabPane = new TabPane(); >> tabPane.getTabs().addAll(tab1, tab2); >> >> TabPaneSkin skin = new TabPaneSkin(tabPane); >> // set overflow menu factory with the same method as was used to >> create the tabs >> skin.setMenuGraphicFactory(this::createGraphic); >> tabPane.setSkin(skin); >> } >> >> // creates graphic Nodes for tabs as well as the overflow menu > ... > > 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 24 additional > commits since the last revision: > > - 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 > - Merge branch 'master' into 8353599.menu.factory > - Merge branch 'master' into 8353599.menu.factory > - ... and 14 more: https://git.openjdk.org/jfx/compare/a0ff2c1a...e4b07cb4 We seem to disagree on a fundamental point, not on whether `null` is a useful default. For you, the default behavior of producing a `Node` for a given `Tab` is not a factory, and only the user-provided function is one. I think this distinction is not justifiable, as there is no difference in principle between the default factory and a user-provided factory. Both take in a `Tab` and produce a `Node` that can be used as a graphical representation for the tab. It doesn't matter if the default factory is limited in functionality, the purpose remains the same. If you are introducing an API for a concept that's already there, the default behavior should be represented using the new API, and it should be given a name and corresponding documentation (which in our case means a public final constant). To make it explicitly clear: I think that `null` is not a useful name for a factory, it's the absence of one; and treating `null` as a sentinel for an actual, real, factory implementation is not a good API design. Whether a `null` sentinel makes it "easier" to revert the factory back to its default is completely besides the point (and it's not even true, a named constant is just as easy to use as a `null` sentinel). It's also a bit of a distraction, because it is exceedingly rare that developers revert control configuration; it is so rare that it should definitely not drive API design. I've read all of the supporting materials and discussions that you provided, and nowhere did anyone (including yourself) say that _reverting_ control configuration is what motivates this proposal. This point just randomly appeared in the discussion a few days ago. The only justifiable API design that would, in my option, warrant using a `null` sentinel is if you named it `menuGraphicFactoryOverride`: in this case, the null value would actually mean that no override is present. But I would not be in favor of adding such a property, as the better alternative is to just give a name to the factory that produces a node for a given tab. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1773#issuecomment-4466183278
