On Fri, 6 Mar 2020 11:47:42 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) > > Ajit Ghaisas has updated the pull request incrementally with one additional > commit since the last revision: > > extra space removed sry for the spelling errors you inherited :) modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewHorizontalArrowsTest.java line 31: > 30: import java.util.function.BiConsumer; > 31: > 32: import org.junit.After; unused import (as you don't parameterize on the keys) modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewHorizontalArrowsTest.java line 90: > 89: sm.setCellSelectionEnabled(false); > 90: > 91: tableView.getItems().setAll("1", "2", "3", "4", "5", "6", "7", > "8", "9", this test is exclusively about cell navigation, which requires cellSelectionEnabled - so should probably do it here (and remove the setting per test method modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewHorizontalArrowsTest.java line 122: > 121: } > 122: > 123: /** changing the parameter of the test is at least unusual, should be doc'ed so future contributors are aware of it modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewHorizontalArrowsTest.java line 152: > 151: public void testForwardSelect() { > 152: sm.setCellSelectionEnabled(true); > 153: sm.select(0, col0); here (and in other test methods) not needed if cellSelection is enabled in setup ------------- Changes requested by fastegal (Author). PR: https://git.openjdk.java.net/jfx/pull/114
