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