RFR: 8294148: Support JSplitPane for instructions and test UI

Harshitha Onkar honkar at openjdk.org
Wed Feb 14 22:43:02 UTC 2024


On Wed, 14 Feb 2024 19:11:42 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> 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.

In terms of flexibility & future extensibility (if any) your approach works better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17845#discussion_r1490152053


More information about the client-libs-dev mailing list