[Rev 04] RFR: 8235480: Regression: [RTL] Arrow keys navigation doesn't respect TableView orientation

Jeanette Winzenburg fastegal at openjdk.java.net
Thu Mar 12 12:44:35 UTC 2020


On Fri, 6 Mar 2020 11:47:42 GMT, Ajit Ghaisas <aghaisas at 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


More information about the openjfx-dev mailing list