[pve-devel] [PATCH widget-toolkit] open picker for all comboboxes on focus, even if they are editable

Dominik Csapak d.csapak at proxmox.com
Thu Apr 16 13:13:46 CEST 2020


when a combobox was editable, a click inside did not open the picker,
but it would if the combobox was not editable.

Since this is
1. inconsistent
2. inconvinient (the user has to specifically press the arrow)

we already had this implemented for our ComboGrid, but not
for regular comboboxes

This patch moves the code for this to an override for ComboBox,
which our ComboGrid then inherits (so we do not need it there anymore)

while at it, do some non-significant code-cleanup
 * whitespace fixes
 * don't shadow 'me' in the focus callback
 * fix typo in comment

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 Toolkit.js        | 31 ++++++++++++++++++++++++++++++-
 form/ComboGrid.js | 23 -----------------------
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/Toolkit.js b/Toolkit.js
index d837890..c30aae1 100644
--- a/Toolkit.js
+++ b/Toolkit.js
@@ -333,7 +333,36 @@ Ext.define('Proxmox.form.ComboBox', {
 	    me.clearValue();
 	    me.setValue(me.originalValue);
 	}
-    }
+    },
+
+    // we also want to open the trigger on editable comboboxes by default
+    initComponent: function() {
+	let me = this;
+	me.callParent();
+
+	if (me.editable) {
+	    // The trigger.picker causes first a focus event on the field then
+	    // toggles the selection picker. Thus skip expanding in this case,
+	    // else our focus listener expands and the picker.trigger then
+	    // collapses it directly afterwards.
+	    Ext.override(me.triggers.picker, {
+		onMouseDown: function(e) {
+		    // copied "should we focus" check from Ext.form.trigger.Trigger
+		    if (e.pointerType !== 'touch' && !this.field.owns(Ext.Element.getActiveElement())) {
+			me.skip_expand_on_focus = true;
+		    }
+		    this.callParent(arguments);
+		}
+	    });
+
+	    me.on("focus", function(combobox) {
+		if (!combobox.isExpanded && !combobox.skip_expand_on_focus) {
+		    combobox.expand();
+		}
+		combobox.skip_expand_on_focus = false;
+	    });
+	}
+    },
 });
 
 // when refreshing a grid/tree view, restoring the focus moves the view back to
diff --git a/form/ComboGrid.js b/form/ComboGrid.js
index 9bdf721..5bea0d0 100644
--- a/form/ComboGrid.js
+++ b/form/ComboGrid.js
@@ -404,29 +404,6 @@ Ext.define('Proxmox.form.ComboGrid', {
             me.createPicker();
         }
 
-	if (me.editable) {
-	    // The trigger.picker causes first a focus event on the field then
-	    // toggles the selection picker. Thus skip expanding in this case,
-	    // else our focus listner expands and the picker.trigger then
-	    // collapses it directly afterwards.
-	    Ext.override(me.triggers.picker, {
-		onMouseDown : function (e) {
-		    // copied "should we focus" check from Ext.form.trigger.Trigger
-		    if (e.pointerType !== 'touch' && !this.field.owns(Ext.Element.getActiveElement())) {
-			me.skip_expand_on_focus = true;
-		    }
-		    this.callParent(arguments);
-		}
-	    });
-
-	    me.on("focus", function(me) {
-		if (!me.isExpanded && !me.skip_expand_on_focus) {
-		    me.expand();
-		}
-		me.skip_expand_on_focus = false;
-	    });
-	}
-
 	me.mon(me.store, 'beforeload', function() {
 	    if (!me.isDisabled()) {
 		me.enableLoadMask = true;
-- 
2.20.1





More information about the pve-devel mailing list