On Tue, 8 Aug 2023 23:44:58 GMT, Jose Pereda <jper...@openjdk.org> wrote:
>> So far, BorderPane does the calculation for the children min/pref >> width/height taken into account only the margin applied to them, if any, but >> not the total padding that could be applied as well to the BorderPane itself. >> >> However, this padding needs to be taken into account as well, and this PR >> modifies BorderPane to subtract its insets from its size while doing the >> children min/pref width/height calculations. >> >> A parameterized test has been included. >> >> It is a simplified version of the test case attached to >> https://bugs.openjdk.org/browse/JDK-8313709, but still shows how without >> this patch, two of the cases (padding with or without margin) fail, while >> pass with it. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Migrate old tests to JUnit 5 modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 395: > 393: > 394: double middleAreaHeight = Math.max(0, > 395: height - insets.getTop() - insets.getBottom() - > topPrefHeight - bottomPrefHeight); should these be snapped? snappedBottomInset() etc.? modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 415: > 413: if (width != -1) { > 414: width -= (insets.getLeft() + insets.getRight()); > 415: } ditto here modules/javafx.graphics/src/test/java/test/javafx/scene/layout/BorderPaneTest.java line 938: > 936: @ParameterizedTest > 937: public void testFlowPaneCenterChildWithPaddingAndMargin(double > width, double minHeight, boolean useMargin, boolean usePadding) { > 938: FlowPane center = new FlowPane(10, 10); I wonder if we should also test the layout-related code using fractional scale + snapping. what do you think? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1203#discussion_r1296446580 PR Review Comment: https://git.openjdk.org/jfx/pull/1203#discussion_r1296447169 PR Review Comment: https://git.openjdk.org/jfx/pull/1203#discussion_r1296450320
