On Tue, 26 Nov 2024 07:04:45 GMT, Abhishek Kumar <[email protected]> wrote:
>> Damon Nguyen has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - String comparison
>> - Review comments
>
> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 60:
>
>> 58: public static void main(final String[] args) throws Exception {
>> 59: AccessibleChoiceTest app = new AccessibleChoiceTest();
>> 60: app.test();
>
> Since you are doing a bit of modifictaion to the test., you may think of
> modifying it further.
> 1. Don't think you need to create the object.
> 2. UI creation code can be moved to more meaningful method name
> `createAndShowUI instead of init` inside EDT block and followed by call to
> `test` method.
Sure. It cleans the test up. Wasn't in my initial plan, but it's a good point.
Done.
> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 102:
>
>> 100:
>> 101: // Focus default button and wait till it gets focus
>> 102: Point loc = def.getLocationOnScreen();
>
> Should be accessed on EDT?
> Frame and other UI components also should be created on EDT ?
Added. Had to make point volatile and static in the process.
> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 129:
>
>> 127: }
>> 128:
>> 129: // Press Down key to select next item in the choice(Motif 2.1)
>
> Motif is deprecated.. can be removed
> Suggestion:
>
> // Press Down key to select next item in the choice
Done
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861394395
PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861392666
PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861392517
