On Mon, 24 Feb 2020 17:15:02 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8235480 >> >> Fix : Added the missed out RTL checks to the key mappings in >> TableViewBehaviorBase class. >> >> Testing : Modified unit tests in TableViewKeyInputTest to take orientation >> as a parameter. The Left/Right key press tests have been modified to address >> LTR and RTL orientations. >> >> Note : If this test modification is acceptable, I would like to address >> other similar tests separately. (I will create a test JBS issue and address >> later) > > The pull request has been updated with 1 additional commit. modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewKeyInputTest.java line 1141: > 1140: keyboard.doLeftArrowPress(KeyModifier.getShortcutKey()); > 1141: } > 1142: keyboard.doKeyPress(KeyCode.SPACE, having such if blocks in all tests regarding horizontal navigation feels like the parametrization isn't quite complete, IMO: the test method shouldn't need to worry, instead just call semantic horizontal navigation methods. A simple change would be to extract that differentiation into a dedicated method, like f.i. forward/backward, test would get simpler (and make it easier to add those that are missing :) @Test public void testForwardFocus() { sm.setCellSelectionEnabled(true); sm.select(0, col0); // current //if (orientation == NodeOrientation.LEFT_TO_RIGHT) { // keyboard.doRightArrowPress(KeyModifier.getShortcutKey()); //} else { // keyboard.doLeftArrowPress(KeyModifier.getShortcutKey()); //} // extracted forward(KeyModifier.getShortcutKey()); assertTrue("selected cell must still be selected", sm.isSelected(0, col0)); assertFalse("next cell must not be selected", sm.isSelected(0, col1)); TablePosition focusedCell = fm.getFocusedCell(); assertEquals("focused cell must moved to next", col1, focusedCell.getTableColumn()); } /** * Orientation-aware horizontal navigation with arrow keys. * @param modifiers the modifiers to use on keyboard */ protected void forward(KeyModifier... modifiers) { if (orientation == NodeOrientation.LEFT_TO_RIGHT) { keyboard.doRightArrowPress(modifiers); } else { keyboard.doLeftArrowPress(modifiers); } } ------------- PR: https://git.openjdk.java.net/jfx/pull/114
