RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v11]
arapte at openjdk.java.net
Fri Sep 11 09:40:22 UTC 2020
On Wed, 9 Sep 2020 11:49:55 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:
>> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
>> rename tests
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ListViewBehavior.java line 87:
>> 85: Predicate<KeyEvent> pIsInComboBox = e -> isListViewOfComboBox != null;
>> 86: Predicate<KeyEvent> pIsInEditableComboBox =
>> 87: e -> isListViewOfComboBox != null && isListViewOfComboBox.get();
> nit-pick about naming: I think we don't use (crippled) hungarian notation, or do we? If we don't, the leading "p"
> should be removed, isIn/Editable/Combo is expressive enough (and no need to differentiate by type of functional
> interface, IMO)
Even I was bit skeptical about prefix p, before it those names sounded like a boolean. So I wanted to give it a
different name. But as you said, isIn/Editable/Combo looks enough expressive. Changed names.
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxListViewSkin.java line 509:
>> 507: getProperties().put("selectFirstRowByDefault", false);
>> 508: getProperties().put("editableComboBoxEditor", (Supplier<Boolean>) () ->
>> getSkinnable().isEditable()); 509: }
> nit-pick about naming: it's the editable state of the combo (vs. the editable state of the editor) that's in the center
> of interest, so maybe rename the key to express that? Like "editableComboBox"?
> Another thingy (which I think is a bit under-used in the fx code-base;) - how about a code comment to why this marker
> is added and which collaborator is using it?
Changed to `editableComboBox` and added a short two liner comment about it.
More information about the openjfx-dev