On Fri, 20 Sep 2024 04:02:12 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> typo
>
> test/jdk/java/awt/Focus/AltTabEventsTest.java line 61:
>
>> 59: 2. Press Alt-tab.
>> 60: In the messqge dialog area you should see that
>> 61: WINDOW_DEACTIVATED,WINDOW_LOST_FOCUS event were generated.
>
> Suggestion:
>
> WINDOW_DEACTIVATED, WINDOW_LOST_FOCUS event were generated.
ok
> test/jdk/java/awt/Focus/AltTabEventsTest.java line 80:
>
>> 78: .rows((int) INSTRUCTIONS.lines().count() + 5)
>> 79: .columns(35)
>> 80: .testUI(AltTabEventsTest::createTestUI)
>
> can be changed to `new Test();` and thus leading to remove the
> `createTestUI()` method.
ok
> test/jdk/java/awt/Focus/AltTabEventsTest.java line 104:
>
>> 102: WindowAdapter wa = new WindowAdapter() {
>> 103: public void windowActivated(WindowEvent e) {
>> 104: println(e.toString());
>
> can be replaced with `PassFailJFrame.log` everywhere. Anyways there is a
> repetition of `println`.
> Suggestion:
>
> PassFailJFrame.log(e.toString());
it will be same..
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 54:
>
>> 52:
>> 53: public ComponentLostFocusTest() {
>> 54: try{
>
> Suggestion:
>
> try {
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 56:
>
>> 54: try{
>> 55: r = new Robot();
>> 56: } catch(Exception e){
>
> Suggestion:
>
> } catch (Exception e) {
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 69:
>
>> 67: });
>> 68:
>> 69: frame.setLayout (new FlowLayout ());
>
> Suggestion:
>
> frame.setLayout (new FlowLayout());
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 79:
>
>> 77: public void doTest() {
>> 78: // Do requesting focus to the modal dialog in order to after that
>> 79: // to do requesting focus to the frame
>
> Doesn't sound correct, can it be rephrased?
removed
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 103:
>
>> 101: tf.addFocusListener(new FocusAdapter() {
>> 102: public void focusGained(FocusEvent e) {
>> 103: System.out.println("TextField gained focus: "+e);
>
> Suggestion:
>
> System.out.println("TextField gained focus: " + e);
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 111:
>
>> 109:
>> 110: System.out.println("Focused window: " +
>> KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusedWindow());
>> 111: System.out.println("Focus owner: " +
>> KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner());
>
> please limit it to 80 cols.
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 118:
>
>> 116: }
>> 117:
>> 118: private void doRequestFocusToTextField(){
>
> Suggestion:
>
> private void doRequestFocusToTextField() {
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 121:
>
>> 119: // do activation using press title
>> 120: Point loc = frame.getLocationOnScreen();
>> 121: r.mouseMove(loc.x + frame.getWidth()/2, loc.y +
>> frame.getInsets().top/2);
>
> Suggestion:
>
> r.mouseMove(loc.x + frame.getWidth() / 2, loc.y +
> frame.getInsets().top / 2);
ok
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 121:
>
>> 119: // do activation using press title
>> 120: Point loc = frame.getLocationOnScreen();
>> 121: r.mouseMove(loc.x + frame.getWidth()/2, loc.y +
>> frame.getInsets().top/2);
>
> Should it be accessed on EDT?
modified test to put relevant portion under EDT and also added dispose..
> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 131:
>
>> 129: }
>> 130:
>> 131: public static final void main(String args[]){
>
> Suggestion:
>
> public static final void main(String args[]) {
ok
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992547
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992609
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992742
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993053
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992807
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992856
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992946
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993001
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993094
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993136
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993175
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993524
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993207
