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