On Fri, 4 Nov 2022 20:42:07 GMT, Andy Goryachev <[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. > > 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. JUnit5 classes do not need to be public. Sure, I will add javadoc. :) > 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? I don't think so. We could add more but I don't see how this will give us any more information/benefits. > 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? Sure, will do. ------------- PR: https://git.openjdk.org/jfx/pull/936
