RFR: 8264127: ListCell editing status is true, when index changes while editing

[Florian Kirmaier]

On Tue, 6 Apr 2021 13:30:46 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> @kleopatra 
>> I've added another test for the case, when index changes in such a way, that the cell should no longer be in the editing state,
>> and the test seems to work. Do I miss something?
>> 
>> About the Events, when i think about it, I was a bit of biased so I don't have to change anything. So I don't really have an opinion about it. But the PR should only fix the case, which is obviously wrong, and i don't really want to change anything in the event logic.
>
>> @kleopatra
>> I've added another test for the case, when index changes in such a way, that the cell should no longer be in the editing state,
>> and the test seems to work. Do I miss something?
> 
> hmm .. that's weird:  your test (cell 1 -> listEdit 0 -> cell 0) indeed passes, doing it the other way round (cell 0 -> listEdit 1 -> cell 1) fails
> 
>     @Test
>     public void testChangeCellIndex0ToListEditingIndex1() {
>         assertChangeIndexToEditing(0, 1);
>     }
>     
>     @Test
>     public void testChangeCellIndex1ToListEditingIndex0() {
>         assertChangeIndexToEditing(1, 0);
>     }
>    
>     private void assertChangeIndexToEditing(int initialCellIndex, int listEditingIndex) {
>         list.setEditable(true);
>         cell.updateListView(list);
>         cell.updateIndex(initialCellIndex);
>         list.edit(listEditingIndex);
>         assertEquals("sanity: list editingIndex ", listEditingIndex, list.getEditingIndex());
>         assertFalse("sanity: cell must not be editing", cell.isEditing());
>         cell.updateIndex(listEditingIndex);
>         assertEquals("sanity: index updated ", listEditingIndex, cell.getIndex());
>         assertEquals("list editingIndex unchanged by cell", listEditingIndex, list.getEditingIndex());
>         assertTrue(cell.isEditing());
>     }
>     
> Something obvious that I missed or a bummer in the test setup or what else? Any ideas?

I've looked into it. 
This is somehow linked to the fact, that the default focusIndex for the list is 0 instead of -1.
If you change the updateDefaultFocus methods in ListView, then it works.

The tests are somehow synthetic and don't reflect the real world.
So it's probably ok to leave it as it is. A better test which reflects the real world would be better, but that's probably way harder. 
Then we would have to get the ListCells from the ListView ... Which would basically be reasonable for all of these tests, but i don't think it should be done as part of this PR.

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

PR: https://git.openjdk.java.net/jfx/pull/441