On Tue, 6 Apr 2021 13:30:46 GMT, Jeanette Winzenburg <faste...@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
