[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8264127: ListCell editing status is true, when index changes while editing
From:       Florian Kirmaier <fkirmaier () openjdk ! java ! net>
Date:       2021-03-31 9:28:09
Message-ID: 4sMuGaAW87f-lxLMt-13WyjuGsEZpuv3SAtgZnKd-VI=.17ae6bc1-b071-46eb-80c1-6216909d0642 () github ! com
[Download RAW message or body]

On Fri, 26 Mar 2021 12:26:53 GMT, Jeanette Winzenburg <fastegal@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


[prev in list] [next in list] [prev in thread] [next in thread]