RFR: 8264127: ListCell editing status is true, when index changes while editing [v6]

Florian Kirmaier fkirmaier at openjdk.java.net
Fri Apr 23 17:06:56 UTC 2021


On Thu, 22 Apr 2021 11:44:41 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8264127:
>>   Fixed another index case based on code review
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line 549:
> 
>> 547:                 cancelEdit();
>> 548:             }
>> 549:         } else {
> 
> minor: not a big fan of local fields, but .. if there already _is_ a local field, we should use it instead of calling the method again (here: editing vs. isEditing) 
> 
> minor: the code comment does no longer fit the code
> 
> major: this will change the editing state of the list - the call to cancelEdit must be wrapped into un/setting the updateEditingIndex flag (see the else-if block later in the method)

done

> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java line 845:
> 
>> 843:             events.add(e);
>> 844:         });
>> 845:         list.setEditable(true);
> 
> missing setup of focus to -1

I've removed this test in favor of the test-method below.

> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java line 911:
> 
>> 909:         assertTrue("sanity: cell must be editing", cell.isEditing());
>> 910:         cell.updateIndex(-1); // change cell index to negative
>> 911:         assertFalse("cell must not be editing if cell index is " + cell.getIndex(), cell.isEditing());
> 
> missing: assert that a cancelEvent is fired and the list editing state is unchanged (they fail until updateEditing is fixed)

The check of cancel-events is now added.

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

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


More information about the openjfx-dev mailing list