On Thu, 9 Apr 2026 22:40:43 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 22 additional 
> commits since the last revision:
> 
>  - 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
>  - Merge branch 'master' into 8353599.menu.factory
>  - Merge branch 'master' into 8353599.menu.factory
>  - ... and 12 more: https://git.openjdk.org/jfx/compare/6995e7f0...914b260a

I agree with the shape of the `menuGraphicFactory` property, but I disagree 
that this should be thought of as an "override" rather than what it says it is: 
It's a factory that takes in a tab and produces an overflow menu graphic, just 
like the default factory does. There's no material difference between the 
default factory and any other custom factory.

This is why, when you give the overflow-menu factory an API, you should also 
give the default factory an API element. Treating `null` as a toggle for an 
actual factory implementation is just not a good choice, and we shouldn't do 
that.

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

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

Reply via email to