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