RFR: 8209788: Left/Right/Ctrl+A keys not working in editor of ComboBox if popup showing [v11]

[Ambarish Rapte]

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.

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

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