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

[Florian Kirmaier]

On Thu, 1 Apr 2021 14:41:46 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> 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.
>
>> 
>> 
>> 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.
> 
> well, I see your point but think it's arguable :)
> 
> - editEvent not/firing on cell re-use is not really specified (even overall edit spec is .. suboptimal, see [my oldish summary](https://github.com/kleopatra/swingempire-fx/wiki/CellEditEvents)) - without that spec, we might feel free to fire 
> - from the perspective of edit handlers, that editCancel (and the assumed editStart - suspect it doesn't happen, didn't test though) are just spurious events - they are not really interested in volatile state changes, just introduced by an implementation detail like cell re-use. 
> 
> As to the scope of this: as I understand it, its goal is to keep the cell's editing state (flag and visual state) in sync with listview's editingIndex when updating the cell index. That should include both directions
> 
> - index == editingIndex -> index != editingIndex
> - index != editingIndex -> index == editingIndex
> 
> The first is already handled (modulo our disagreement on editEvents :), the second is not.

@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.

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

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