On Fri, 28 Apr 2023 21:33:54 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Monkey Tester - a JavaFX application designed to support manual ad-hoc 
>> testing of individual JavaFX controls.  Unlike Ensemble, the goal of this 
>> application is to facilitate manual testing rather than demonstrate the 
>> capabilities of JavaFX.
>> 
>> Feedback and suggestions are always welcome.
>> 
>> ![screenshot](https://user-images.githubusercontent.com/107069028/232911797-3d02da68-ce11-419e-8f16-c2661b778f9c.png)
>
> Andy Goryachev has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   readme

This is looking really good. I'll be glad to see this get in soon. I'll finish 
my testing before long.

Here are a few global comments. I left several, mostly minor, inline comments. 
The only "must fix" items are the two files with the missing copyright header.

Build:

* Suggestion: Can you set the default value of `javafx.home` to 
`../../../build/sdk`? That way the developer won't need to specify it in the 
common case of wanting to build and test the repo you are building it from.


Testing:

* On my system, the window initially comes up with the top-level SplitPane too 
far to the left (see image below)

* When I relaunch the app, it remembers the position and size of the window, 
but the top-level SplitPane is resized so that it sometimes clips part of the 
demo list on the left

* Possible future RFE: Add an options to reset all settings to their defaults? 
Given the `-Ddisable.settings=true` option, this seems like a low priority RFE 
(if it's even necessary).

* Food for thought: I can see the value of it remembering the size and location 
of my window, and the last test panel I was on, but I'm less likely to want it 
to remember every knob that I've changed while doing some testing. This begs 
the question: what settings should be considered "sticky" (global changes) 
versus not sticky (session changes)? It's something that could be tweaked later 
if there was a desire to do so.


Code:

* Very minor suggestion: I see a lot of unnecessary uses of "protected" in the 
individual pages classes. Since this is a standalone test program it doesn't 
matter, but I'd guess most of them could be default "package scope" access. 
Probably not worth doing.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1097#pullrequestreview-1406552771

Reply via email to