On Sat, 17 Jul 2021 00:15:48 GMT, Jose Pereda <[email protected]> wrote:
> As described in the issue, when the stage is moved to the screen top
> position, it is moved below the system menu bar. However, doing it twice
> doesn't trigger a native callback to the Java layer, and the stage yPosition
> doesn't get updated with the actual position of the application. This has
> several side effects, like the wrong popup control position.
>
> This PR adds a callback in case set position doesn’t match actual position.
>
> It is only going to be called when the final position of the stage doesn't
> match the one that was set, which could happen in rare occasions, mainly due
> to constrains applied by the native layer, so it doesn't add any overhead.
>
> A system test for MacOS is included.
The fix and the test looks good. I left one suggestion and two minor formatting
comments on the test.
tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line
25:
> 23: * questions.
> 24: */
> 25: package test.javafx.stage;
Minor: can you add a blank line before the `package` statement? (I realize we
have a few existing classes that don't follow this)
tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line
72:
> 70: stage.setY(400);
> 71: stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e ->
> 72:
> Platform.runLater(startupLatch::countDown));
Minor: indentation is a bit off if you intended to line it up.
tests/system/src/test/java/test/javafx/stage/StageAtTopPositionTest.java line
86:
> 84: }
> 85: } catch (InterruptedException ex) {
> 86: fail("Unexpected exception: " + ex);
If you add `throws Exception` to the method, you can simplify this and replace
the try/catch block with:
assertTrue("Timeout waiting for FX runtime to start",
startupLatch.await(15, TimeUnit.SECONDS));
(most of our newer tests do this)
-------------
PR: https://git.openjdk.java.net/jfx/pull/580
