On Thu, 22 Jul 2021 19:15:54 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>>> > just checked my notes (there's a cell-editing branch in my fork where I'm 
>>> > experimenting) - astonishingly the answer is no, could not see anything 
>>> > :) And actually, seems like we don't even need to return immediately: 
>>> > would have expected some unhealthy side-effects on doing the switching 
>>> > into visual editing state more than once, but couldn't detect anything. 
>>> > You might try, though :)
>>> 
>>> Okay. Question is, should we guard against a double edit? There is already 
>>> one in `TreeTableCell#startEdit`, but probably forgotten in TableCell. I 
>>> think it makes sense and as there is already the check in TreeTableCell, 
>>> there was at least a thought of it somewhere in the past.
>> 
>> good question, next question ;) 
>> 
>> - the oversight in startEdit of the base List/TableCell is not part of this 
>> (covered and soon fixed by 
>> [JDK-8188027](https://bugs.openjdk.java.net/browse/JDK-8188027), the 
>> concrete misbehavior is that they fire multiple edit events
>> - as to the "real" editing cell types (that is those that actually have an 
>> editingComponent) -  we (that is now you *grin) should try hard to find a 
>> scenario where multiple starts (== multiple configuration passes of the 
>> editingComponent) might hurt. Like when the user already typed something and 
>> for some reason startEdit is called again, the configuration would delete 
>> the input. 
>> 
>>  > If there is nothing left, should I create a ticket for `startEdit` and 
>> for `cancelEdit` (this only affects the sub classes) ? :)
>> 
>> hmm - not sure I understand what you are asking: startEdit is covered, and 
>> cancelEdit would be similar - either you find a scenario where multiple 
>> un-configure of the cell (after cancel) would hurt or not?
>
>> > > just checked my notes (there's a cell-editing branch in my fork where 
>> > > I'm experimenting) - astonishingly the answer is no, could not see 
>> > > anything :) And actually, seems like we don't even need to return 
>> > > immediately: would have expected some unhealthy side-effects on doing 
>> > > the switching into visual editing state more than once, but couldn't 
>> > > detect anything. You might try, though :)
>> > 
>> > 
>> > Okay. Question is, should we guard against a double edit? There is already 
>> > one in `TreeTableCell#startEdit`, but probably forgotten in TableCell. I 
>> > think it makes sense and as there is already the check in TreeTableCell, 
>> > there was at least a thought of it somewhere in the past.
>> 
>> good question, next question ;)
>> 
>> * the oversight in startEdit of the base List/TableCell is not part of this 
>> (covered and soon fixed by 
>> [JDK-8188027](https://bugs.openjdk.java.net/browse/JDK-8188027), the 
>> concrete misbehavior is that they fire multiple edit events
>> * as to the "real" editing cell types (that is those that actually have an 
>> editingComponent) -  we (that is now you *grin) should try hard to find a 
>> scenario where multiple starts (== multiple configuration passes of the 
>> editingComponent) might hurt. Like when the user already typed something and 
>> for some reason startEdit is called again, the configuration would delete 
>> the input.
>> 
>> > If there is nothing left, should I create a ticket for `startEdit` and for 
>> > `cancelEdit` (this only affects the sub classes) ? :)
>> 
>> hmm - not sure I understand what you are asking: startEdit is covered, and 
>> cancelEdit would be similar - either you find a scenario where multiple 
>> un-configure of the cell (after cancel) would hurt or not?
> 
> Finally coming back to this, when firing a **startEdit()** while already 
> editing e.g. a TextFieldCell, the input which was made by you gets lost and 
> you need to start over. So this can be a potential follow-up. I didn't found 
> anything for **cancelEdit()**.

@Maran23 wondering why my review comments aren't addressed? Anything unclear?

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

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

Reply via email to