On Tue, 23 Apr 2024 07:51:59 GMT, Tejesh R <t...@openjdk.org> wrote:

>> Instructions set has been updated as per OS specific. JTable keyboard 
>> navigation is tested in each OS and according it's current implementation 
>> the instructions has been updated (Few has been removed and few has been 
>> updated). 
>> PassFailJFrame.builder is used.
>
> Tejesh R has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review updates

Default time out may not be sufficient to test, can be increased.

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 198:

> 196:         final String WINDOWS_SPECIFIC = """
> 197:                 Tab, Shift-Tab - Navigate In.
> 198:                 Return/Shift-Return - move focus one cell down/up.

Suggestion:

                Return/Shift-Return - Move focus one cell down/up.

For consistency please ensure each command action to start with either lower 
case or upper case. Same for F2, Esc commands etc.

Check for Linux and Mac specific instructions also.

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 202:

> 200:                 Up/Down Arrow - deselect current selection; move focus 
> one
> 201:                                 cell up/down
> 202:                 Left/Right Arrow - deselect current selection; move focus

Seems the instruction needs to modify.
On press of Left/Right arrow, only focus shifted to one cell left or right but 
the current row selection remains same.

Similar for Home/End as well, current selection remains as it is.

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 217:

> 215:                 F2 - Allows editing in a cell containing information 
> without
> 216:                      overwriting the information
> 217:                 Esc -  Resets the cell content back to the state it was 
> in

Suggestion:

                Esc -  Reset the cell content back to the state it was in.


Minor suggestion,  end with `.` for each statement else nothing.

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 220:

> 218:                        before editing started
> 219:                 Ctrl+A, Ctrl+/ - Select All
> 220:                 Ctrl+\\ - De-select all

Suggestion:

                Ctrl+\\ - deselect all

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 222:

> 220:                 Ctrl+\\ - De-select all
> 221:                 Shift-Up/Down Arrow -  extend selection up/down one row
> 222:                 Shift-Left/Right Arrow - extend selection left/right one

No extended selection of columns only focus changed from one cell to another.

test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 226:

> 224:                 Control-shift Up/Down Arrow -  extend selection to 
> top/bottom
> 225:                                                 of column
> 226:                 Shift-Home/End -  extend selection to left/right end of 
> row

No extended selection only focus shifted to first and last column.

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

PR Review: https://git.openjdk.org/jdk/pull/18855#pullrequestreview-2022085343
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579228099
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579218399
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579221288
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579231419
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579219557
PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579220991

Reply via email to