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/aa924f8f...e4b07cb4

`menuGraphicFactoryOverride` might be a better name, I agree - that what it is 
designed to be, an override.

There is one possible use case which favors a factory over an override: when 
the application is too lazy to implement the whole factory, wanting instead to 
handle the custom tabs, delegating the other kinds to the default factory.  Not 
the use case I initially envisioned, but it is a possibility that suggests 
exposing a `DEFAULT_MENU_GRAPHIC_FACTORY` might be a better approach. 

One more point I wanted to bring up is the use of `Callback` instead of a 
dedicated interface - we had this problem when adding table column resize 
policies.  A `Callback` cannot be evolved, while a dedicated interface can, 
using default methods.  I don't think this should be the case here since the 
functionality is very limited, but I want to bring it to your attention.

Given the popular feedback, I'll update the PR to expose the 
`DEFAULT_MENU_GRAPHIC_FACTORY`.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1773#issuecomment-4479029250

Reply via email to