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? Everything clear, I was just busy the last days. But I think I will address everything in this week. If you can't continue without it e.g. this PR is blocking your (future) work, you can also continue on this. :) ------------- PR: https://git.openjdk.java.net/jfx/pull/569
