On Thu, 1 Apr 2021 14:41:46 GMT, Jeanette Winzenburg <faste...@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

Reply via email to