On Sat, 17 Jul 2021 00:15:48 GMT, Jose Pereda <jper...@openjdk.org> 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

Reply via email to