Interesting idea!

I had some concern about disabling the overflow menu - accessibility - but 
actually it is still possible to navigate to hidden tabs with the arrow keys.  
The problem that creates is we would need some sort of indicator that some tabs 
are hidden, which defeats the purpose.

Since the main complaint was that custom graphic Nodes are not rendered 
(especially when tabs have no text), fixing that might reduce the need for 
other changes.
(I also want to mention that the case of table button menu is very different, 
as it's an optional feature, but the overflow menu is an essential part of the 
current tab pane skin).

-andy


From: Marius Hanl <mariush...@web.de>
Date: Friday, April 11, 2025 at 12:14
To: Andy Goryachev <andy.goryac...@oracle.com>, credm...@certak.com 
<credm...@certak.com>
Cc: openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
Subject: Aw: Re: [External] : Re: TabPane overflow menu showing blanks
I wonder if we want to implement something similar in the TabPaneSkin as we did 
for the TableColumnHeader Column Menu.
See also: https://bugs.openjdk.org/browse/JDK-8091153 and the PR: 
https://github.com/openjdk/jfx/pull/1135<https://urldefense.com/v3/__https:/github.com/openjdk/jfx/pull/1135__;!!ACWV5N9M2RV99hQ!LAnN8cAz3_lF-gPNePLRXsG1BSwuUbcXqcqZYhJhkr8nMhggyA-_MO9g63jCWcSc6h5CrRZ1V_TBI-8pTOHQw5Ek$>
In a former PR, I made the creation of the menu lazily, so if the 
showColumnMenu(..) method is overridden, no resources and listener will ever be 
created/used for the JavaFX Column Menu.

If we would do the same in TabPaneSkin, then everyone can create an 
implementation where the whole logic is up to the developer - showing a custom 
menu, something else, or nothing.

-- Marius

Gesendet: Mittwoch, 9. April 2025 um 20:25
Von: "Andy Goryachev" <andy.goryac...@oracle.com>
An: "Cormac Redmond" <credm...@certak.com>
CC: "openjfx-dev@openjdk.org" <openjfx-dev@openjdk.org>
Betreff: Re: [External] : Re: TabPane overflow menu showing blanks
No problem, Cormac, and thank you for suggestions!  I guess you will be 
volunteering for the code review?  (No promises as to when this feature will be 
available though).

Speaking of the disabling the overflow menu: this may not be a good idea from 
the accessibility perspective, but the idea of menu item factory looks quite 
promising.  Since we are on the subject, there is one more possibility - 
Tab::setGraphicProvider(Supplier<Node>) instead of a menu item factory, in 
addition to the existing Tab::setGraphic(Node).  Might be less flexible as one 
won't be able to create custom menu items.  Just a thought.

-andy


From: Cormac Redmond <credm...@certak.com>
Date: Tuesday, April 8, 2025 at 15:49
To: Andy Goryachev <andy.goryac...@oracle.com>
Cc: openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
Subject: [External] : Re: TabPane overflow menu showing blanks
Hi Andy,

Thank you for your reply.

That sounds good, re: a menu item factory. E.g., give the devs full control.

I think the ability to disable it (or have it disabled by default) would be 
good too: a dev might not care about supporting the menu items in many cases. 
E.g., if a TabPane only has three (graphic-based) tabs, and the user decides to 
resize the window to something unusually small, then who cares if they can't 
traverse the tabs, at that size? Nobody realistically expects to be able in 
such a scenario anyway, and it's better to have no menu than a buggy one.

I don't like the fact that, today, TabPane's menu can misbehave and nobody 
would know it's going to happen really. I think TabPane should throw an 
exception (or log a warning) when it receives nodes it knows it will not be 
able to use in the menu; especially given the menu is always-on.

I think a good overall solution would be:
1.   a menu factory
2.   make the menu optional for those who don't care
3.   log warnings when a tab's header node will not be displayable in a menu 
(i.e., if the dev hasn't provided a factory and the default "menu factory" 
knowingly cannot handle it)
1.   Not really sure how JFX folks feel about logging warnings, I note it's 
minimal today in general, and probably for good reason (e.g., where do you draw 
the line in the sand about what should be logged vs. not)
Anyway, I think the above will be achievable without any breaking / 
functionality changes...?



Kind Regards,
Cormac

On Mon, 7 Apr 2025 at 17:59, Andy Goryachev 
<andy.goryac...@oracle.com<mailto:andy.goryac...@oracle.com>> wrote:
All very good points, thank you for this writeup!

This discussion relates to https://bugs.openjdk.org/browse/JDK-8353599 .  I've 
been thinking how to handle this issue, and I am leaning to agree with the 
suggestion proposed in the ticket (some sort of menu item factory).

The current "hacky" solution not obviously fails, but also not scalable - I can 
see developers wanting to place a Canvas, a Path, a Pane, or any other kind of 
Node and it would be nearly impossible to "clone" these in a maintainable 
manner.  With a menu item factory, however, the effort will be shifted on the 
application dev with a very simple solution.

The other solution that does not involve a new API would be to limit the 
overflow menu to only list the tabs that are hidden - in this case the graphics 
Nodes can be reused by the menu items.  The problem with that approach is the 
overflow menu logic becomes more complicated, to handle the case when menus are 
hidden on both ends.

What do you think?

-andy


From: openjfx-dev 
<openjfx-dev-r...@openjdk.org<mailto:openjfx-dev-r...@openjdk.org>> on behalf 
of Cormac Redmond <credm...@certak.com<mailto:credm...@certak.com>>
Date: Friday, April 4, 2025 at 17:00
To: openjfx-dev@openjdk.org<mailto:openjfx-dev@openjdk.org> 
<openjfx-dev@openjdk.org<mailto:openjfx-dev@openjdk.org>>
Subject: TabPane overflow menu showing blanks
Hi,

TabPane tabs allow you to set graphic nodes as the header and there appears to 
be no documented limitations or best-practises on this.

You might assume it's perfectly reasonable to not set a Tab's text value, and 
instead set the header as a HBox, consisting of a graphic node (left) and a 
Label (itself with a text + a graphic, right), to achieve an icon-text-icon 
style header, which is not an uncommon.

However, the overflow menu, when it kicks in, will not display any text or 
graphics at all (just a blank space), if your Tab has no "text" set, and the 
header graphic is not a Label or an ImageView.

It's down to this self-confessed hacky code 
(https://github.com/openjdk/jfx/blob/f31d00d8f7e601c3bb28a9975dd029390ec92173/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java#L479<https://urldefense.com/v3/__https:/github.com/openjdk/jfx/blob/f31d00d8f7e601c3bb28a9975dd029390ec92173/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java*L479__;Iw!!ACWV5N9M2RV99hQ!Mran6HXmukSt2WDoAR19oIpx2dUYe7SVW7YGm6LeVeGvjzbRIBG_bZU5PLxtBI6BiE7zFepT1Vk08YpHig6pbw$>):

    /**
     * VERY HACKY - this lets us 'duplicate' Label and ImageView nodes to be 
used in a
     * Tab and the tabs menu at the same time.
     */
    private static Node clone(Node n) {
        if (n == null) {
            return null;
        }
        if (n instanceof ImageView) {
            ImageView iv = (ImageView) n;
            ImageView imageview = new ImageView();
            imageview.imageProperty().bind(iv.imageProperty());
            return imageview;
        }
        if (n instanceof Label) {
            Label l = (Label)n;
            Label label = new Label(l.getText(), clone(l.getGraphic()));
            label.textProperty().bind(l.textProperty());
            return label;
        }
        return null;
    }


It is not obvious at all that this is what's going to happen, or why. And it 
seems impossible to control or influence this in any reasonable way, without 
replacing the whole TabPaneSkin.

Given TabPane is one of the most important and widely used controls there is, 
there could/should be some of the following:

  *   Proper documentation on this limitation / behaviour
  *   Some way to set fallback text that can be used in the menu (i.e., that 
isn't the Tab's header's text, because one might intentionally avoid setting 
that given there's no way to hide it from the tab header)
  *   Or, better yet, some way to allow you to specify tab header text without 
displaying it in the actual tab header, which could then be used by the hacky 
method as normal
  *   Some way to set a callback so the developer can decide what gets 
displayed for the overflow menu
  *   Some way to override the skin without needing a full copy + paste + rename
  *   Some way to allow the dev to disable the overflow menu, if it's going to 
produce something unusable. E.g., prefer no menu than a buggy one...

I would view this as a bug, even though it's probably been like this forever.


Some sample code to demonstrate; just look at the menu:

public class TabPaneMenu extends Application {
    @Override
    public void start(Stage primaryStage) {
        TabPane tabPane = new TabPane();

        for (int i = 1; i <= 30; i++) {
            Tab tab;
            if (i == 2) {
                tab = new Tab();
                final Label label = new Label("HBox Tab " + i);
                final HBox hBox = new HBox(label); // Will show as blank in 
overflow menu!
                tab.setGraphic(hBox);
            } else if (i == 5) {
                tab = new Tab();
                tab.setGraphic(new Label("Label Tab " + i)); // Will show in 
overflow menu, because it's a Label
            } else {
                tab = new Tab("Normal Tab " + i);
            }

            tab.setContent(new StackPane(new Label("This is Tab " + i)));
            tab.setClosable(false);
            tabPane.getTabs().add(tab);

        }

        Scene scene = new Scene(tabPane, 800, 600);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    public static void main(String[] args) {
        launch(args);
    }
}


Kind Regards,

Cormac Redmond
Software Engineer, Certak Ltd.

e: credm...@certak.com<mailto:credm...@certak.com> | m: +353 (0) 86 268 2152 | 
w: 
www.certak.com<https://urldefense.com/v3/__http:/www.certak.com__;!!ACWV5N9M2RV99hQ!Mran6HXmukSt2WDoAR19oIpx2dUYe7SVW7YGm6LeVeGvjzbRIBG_bZU5PLxtBI6BiE7zFepT1Vk08YpVdP5RVQ$>


Reply via email to