On Wed, 7 Oct 2026 15:26:23 GMT, Andy Goryachev <[email protected]> wrote:
>> This change fixes missing graphic in the `TabPane` overflow menu, using a
>> different approach. Instead of providing a graphic override property, it
>> now uses a "decorator" pattern to allow for wider customization of the
>> overflow menu (following @Maran23 's suggestion).
>>
>>
>> ### Summary of Changes
>>
>> - minor `TabPaneSkin` constructor javadoc clarification
>> - added the `overflowMenuDecorator` property
>> - changed popup menu to be created on demand
>> - avoid adding the popup reference to the `TabHeaderSkin.properties` (I
>> think it was done for testing purposes, but I could not find any references
>> to it in the platform code)
>>
>>
>> ### Question for the Reviewers
>>
>> 1. do we want to bind the `MenuItem.text` property if decorator is set?
>>
>>
>>
>> # Overflow Menu Decorator Property in the TabPaneSkin
>>
>> ## Summary
>>
>> Add the `overflowMenuDecorator` property in the `TabPaneSkin` class to allow
>> for customization of
>> the `TabPane` overflow menu, including support for custom graphic.
>>
>>
>>
>> ## Problem
>>
>> The original issue described impossibility of supporting custom graphic
>> (such as `Path` or `Canvas`)
>> in the `TabPane` overflow menu.
>> This happens because
>> the overflow menu must duplicate the the graphic, and the current
>> implementation is limited to duplicating
>> either `ImageView` or `Label` with an `ImageView` graphic.
>>
>> Going beyond the original complaint, it is also impossible to customize the
>> overflow menu, when, for example,
>> the application requirements call for text-only overflow menu (with tabs
>> that include the graphic), or when
>> the overflow menu needs to contain different text/graphic, or apply
>> different rules to individual menu items.
>>
>>
>>
>> ## Solution
>>
>> The solution is to allow the application to set a "decorator" which would be
>> used to customize the menu items
>> in the overflow menu (the actual menu item instances are of certain type
>> created by the skin).
>>
>> The `overflowMenuDecorator` property holds the decorator instance. The
>> default `null` value makes the `TabPane`
>> work exactly as it works now.
>>
>> Example:
>>
>>
>> public class TabPaneGraphicFactoryExample {
>> public void example() {
>> Tab tab1 = new Tab("Tab1");
>> tab1.setGraphic(...);
>>
>> Tab tab2 = new Tab("Tab2");
>> tab2.setGraphic(...);
>>
>> TabPane tabPane = new TabPane();
>> tabPane.getTabs().addAll(tab1, tab2);
>>
>> TabPaneSkin skin = new TabPaneSkin(tabPane);
>> // set overflow me...
>
> Andy Goryachev has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 37 commits:
>
> - Merge branch 'master' into 8353599.menu.factory
> - Merge branch 'master' into 8353599.menu.factory
> - Merge branch 'master' into 8353599.menu.factory
> - test
> - cleanup
> - overflow menu decorator
> - Merge remote-tracking branch 'origin/master' into 8353599.menu.factory
> - Merge branch 'master' into 8353599.menu.factory
> - override
> - javadoc
> - ... and 27 more: https://git.openjdk.org/jfx/compare/68b0b112...6449d9d8
I will do some more testing soon. The CSS stuff is really draining my mental
energy right now.
The API is fine for me, although to completely override the `Popup` seems to be
a good idea at one point as well.
So when clicking the overflow button, a `protected` method be called that does
what is currently done, but you can also override and just do a completely
different thing. Lazily creating the `Popup`, as you do now is a good first
step to that too.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1773#issuecomment-6041222367
