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