On Wed, 14 Feb 2024 18:47:55 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> This enhancement adds three methods to `PassFailJFrame.Builder`: >> >> 1. **`splitUIRight`** to display test UI to the right of the instructions; >> 2. **`splitUIBottom`** to display test UI to the bottom of the instructions; >> 3. `splitUI`: a convenience method for a default split orientation, >> currently defaults to `splitUIRight`. >> >> If UI of a test is simple, it's reasonable to embed the test UI into the >> instruction UI. In this case, there's only one frame the tester will be >> interacting with. >> >> The implementation is similar to that of `testUI` provided under >> [JDK-8294156](https://bugs.openjdk.org/browse/JDK-8294156). The `splitUI-` >> method accept a reference to `PanelCreator` interface. Instead of creating >> an entire frame, you create only a component, usually a `JPanel` or `Box`, >> which contains all the test UI. >> >> For example, >> >> >> PassFailJFrame.builder() >> .instructions("Instructions for the test") >> .splitUI(() -> { >> JPanel panel = new JPanel(); >> panel.add(new JButton("Click Me")); >> return panel; >> }) >> .build() >> .awaitAndCheck(); >> >> >> In addition to it, there's an advanced `testUI` method which accepts >> `PanelCreator`; in this case the test UI is hosted in `JDialog` with the >> instruction frame being its owner. Because the test UI is in a modeless >> owned dialog, it always displays above the instruction frame, switch to the >> test app brings up both the instructions and test UI. I don't expect it'd be >> used often, yet it's easy to implement. > > test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1229: > >> 1227: * @throws IllegalArgumentException if {panelCreator} is >> {@code null} >> 1228: */ >> 1229: public Builder splitUI(PanelCreator panelCreator) { > > Suggestion: > > public Builder splitUI(PanelCreator panelCreator, int > splitUIOrientation ) { > > > > Would it be easier if we use `splitUIOrientation` or an enum similar to > [Position](https://github.com/openjdk/jdk/blob/b823fa44508901a6bf39795ab18991d055a71b4e/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java#L191) > to determine the split than have three different methods - `splitUI, > splitUIRight & splitUIBottom`? And eliminate the private `splitUI()` and have > the orientation logic within public `splitUI()` I don't think so… I find the `Position` cumbersome too. Using `JSplitPane.HORIZONTAL_SPLIT` or `JSplitPane.VERTICAL_SPLIT` in a public method of Builder exposes the internal detail that `JSplitPane` is used. What if we decide to implement it differently? After all, the UI could be placed into a horizontal or vertical [`Box`](https://docs.oracle.com/en/java/javase/21/docs/api/java.desktop/javax/swing/Box.html)? Passing an `int` as the orientation is not type-safe, we'll need to verify the parameter or let `JSplitPane` constructor do it. An enum is type-safe but it's cumbersome to type: splitUI(() -> new JPanel(), PassFailJFrame.SplitOrientation.HORIZONTAL) // or splitUI(() -> new JPanel(), JSplitPane.HORIZONTAL_SPLIT) does not look as good to me as splitUIRight(() -> new JPanel()) `-Right` explicitly conveys the chosen position and is much shorter than the above samples with `enum` or constants from `JSplitPane`. Initially, I wanted to re-use `Position` but decided not to … for flexibility. Split UI could be combined with additional test UI windows… Not that I see a use case for such a use… The three methods make the `Builder` more crowded for the sake of brevity of its usage in the test code. I expect it would be `splitUI` in majority of cases. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17845#discussion_r1489943955
