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

[Florian Kirmaier]

On Fri, 26 Mar 2021 12:26:53 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> To clarify, this with happening with ListViews in production code.
>> In some conditions, the status of a single cell doesn't update. The effect is, that a Cell is stuck in the Editing mode.
>> If I remember correctly, it was irreversible stuck. It happened rarely, it might be connected to cells with variable sizes.
>> 
>> Handling the logic from the ListView seems wrong to me, I think the ListCell should update its state automatically dependent of the properties. But I wouldn't know where to add it in the ListView. If that's how it's done in the other cell-based components, then it would make sense and the code more linear. I'm not really opinionated on how to solve it, just went for the simplest fix I've found.
>
>> 
>> Handling the logic from the ListView seems wrong to me, 
> 
> looks like I was unclear, because that's a 100% me-too :) 
> 
> Reformulating my second sentence in test snippets:
> 
>      A:
>      cell.updateIndex(1);
>      list.edit(1);
>      cell.updateIndex(0);
>      // failing/passing before/after fix
>      assertFalse("cell re-use must update cell editing state" , cell.isEditing());
> 
>      B:
>     List<EditEvent> events = new ArrayList<EditEvent>();
>     list.setOnEditCancel(e -> {
>         events.add(e);
>     });
>      .... setup test state as in A
>     // passing/failing before/after fix
>     assertTrue("cell re-use must not trigger edit events", events.isEmpty()); 
>    
>     C:
>      .... setup test state as in A
>      // passing/passing before/after fix     
>      assertEquals("cell re-use must not change list editing state", 1, list.getEditingIndex);
>     
> My question was, whether we agree on B. My wondering was about C passing in the light of B failing.

I don't think B is right.
I would expect a editCancel event when the index of an editing cell is changed.
If there would be another cell, which will get the index 1 (which isn't the case in this artifical test) then i would expect another editStart event.

On the perspective of the ListView, the index never changes, but it's more relevant that some of the cells cancel/start the editing. If one would want to avoid the cancelEdit, when the index is not changing, then the solution should be to make sure, the index of editing cells never changes, which isn't the scope of this PR.

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

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