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