On Sat, 2 Sep 2023 13:09:14 GMT, Marius Hanl <[email protected]> wrote:
>> This PR adds a test that verifies the `SkinBase.layoutChildren(..)` method
>> with different scales.
>> While not explicitly documented, this method will receive the snapped and
>> correctly calculated x, y, width and height values, so that children of the
>> Control can be layouted correctly without requiring to do many more
>> calculations regarding padding.
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JDK-8315569: Set a min size
modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java
line 44:
> 42: import static org.junit.jupiter.api.Assertions.assertTrue;
> 43:
> 44: class ControlContractTest {
Do you plan to add more tests to this class? The name does not specify which
contract is being tested.
modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java
line 46:
> 44: class ControlContractTest {
> 45:
> 46: private ControlStub control;
Do you think it's worth to run this test against all of the existing Controls?
modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java
line 67:
> 65: */
> 66: @ParameterizedTest
> 67: @ValueSource(doubles = { 1.0, 1.25, 1.5, 1.75, 2.0 })
Could you please add 2.25 (I can see this one with the secondary monitor
attached to my win11 box)
modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java
line 101:
> 99:
> 100: double expectedWidth =
> control.snapSizeX(control.getWidth()) - control.snappedLeftInset() -
> control.snappedRightInset();
> 101: assertEquals(expectedWidth, w);
since we are performing floating point operations, we might accumulate small
error.
So this and 3 subsequent assertEquals() would need an EPSILON = 0.0000001 (an
arbitrary value)
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319158123
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319159680
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319150932
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319155511
