On Thu, 3 Nov 2022 21:15:08 GMT, Marius Hanl <[email protected]> wrote:
> While checking https://bugs.openjdk.org/browse/JDK-8295078 I found much more > layout container which do not snap their children correctly. > > The goal of this PR is to add snapping tests for all layout container. > After that we can create issues for all broken layout container, fix them and > comment in the corresponding line. minor questions and suggestions, otherwise looks good. modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 46: > 44: import static org.junit.jupiter.api.Assertions.*; > 45: > 46: class SnappingTest { should it be `public class SnappingTest`? Also, perhaps a brief javadoc explaining the purpose of this test might help. modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 85: > 83: @MethodSource(value = { "getContainerInstruction" }) > 84: void testContainerSnappingScale200(ContainerInstruction<Region> > containerInstruction) { > 85: testContainerSnappingImpl(containerInstruction, 2); minor: do you think there is a need for other scale factors? modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 127: > 125: > 126: assertEquals(widthHeight + snappedPaddingX, > container.prefWidth(-1), 0.00001, className); > 127: assertEquals(widthHeight + snappedPaddingY, > container.prefHeight(-1), 0.00001, className); perhaps this should be a constant? ------------- PR: https://git.openjdk.org/jfx/pull/936
