P4
When creating a TitledPane, it is hard-coded to look only at the preferred size of its label. For normal text labels, this is usually good enough, however, a label can basically be any node by setting its graphic. The graphic can be resizable, and thus layout code needs to respect its minimum, preferred and maximum sizes.
For example, if I set the title to be an HBox of the form (text, gap, graphic), the HBox has a maximum width that differs from its preferred width. Yet, TitledPane will effectively remove the gap as it only looks at preferred width.
Cause:
TitledPane overrides label calculations normally delegated to LabeledSkinBase. It does this because "the padding from TitledPane was being applied to the Label when it should not be". The overridden calculation is simplistic, and does not taken resizable controls into account.
Solution:
Let TitledPane use the calculations from LabeledSkinBase. The padding problem can actually be resolved by overriding the methods that provide the label padding and setting them to 0. This makes the calculation consistent with normal Labeled controls, removes duplicated code and makes resizable labels work correctly.
Further issues (outside scope of this ticket):
I've discovered that a similar bug is present in LabeledSkinBase as it duplicates a lot of logic in order to layout two components (a text and a graphic). It does this to avoid creating a parent container (like an HBox or VBox) when the labeled consists of two distinct pieces (a text and a graphic). However, this logic is too simplistic and fails to take into account that both text and graphic can be resizable. When both are present, it assumes the graphic to be of a fixed size. An HBox or VBox would not do this.
I think in the future it may be wise to make the HBox/VBox logic available statically. LabeledSkinBase can then reuse these calculations without being forced to introduce a container in the scene graph. Alternatively, LabeledSkinBase could just use a container, but I'm assuming this was considered and rejected to avoid creating an extra node.
For example, if I set the title to be an HBox of the form (text, gap, graphic), the HBox has a maximum width that differs from its preferred width. Yet, TitledPane will effectively remove the gap as it only looks at preferred width.
Cause:
TitledPane overrides label calculations normally delegated to LabeledSkinBase. It does this because "the padding from TitledPane was being applied to the Label when it should not be". The overridden calculation is simplistic, and does not taken resizable controls into account.
Solution:
Let TitledPane use the calculations from LabeledSkinBase. The padding problem can actually be resolved by overriding the methods that provide the label padding and setting them to 0. This makes the calculation consistent with normal Labeled controls, removes duplicated code and makes resizable labels work correctly.
Further issues (outside scope of this ticket):
I've discovered that a similar bug is present in LabeledSkinBase as it duplicates a lot of logic in order to layout two components (a text and a graphic). It does this to avoid creating a parent container (like an HBox or VBox) when the labeled consists of two distinct pieces (a text and a graphic). However, this logic is too simplistic and fails to take into account that both text and graphic can be resizable. When both are present, it assumes the graphic to be of a fixed size. An HBox or VBox would not do this.
I think in the future it may be wise to make the HBox/VBox logic available statically. LabeledSkinBase can then reuse these calculations without being forced to introduce a container in the scene graph. Alternatively, LabeledSkinBase could just use a container, but I'm assuming this was considered and rejected to avoid creating an extra node.
- links to
-
Review(master)
openjdk/jfx/1742