Tried my hand at a proper code review, I like the patches in general.

On 9/13/19 3:16 PM, Aaron Lauterer wrote:
Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
---
  www/manager6/Makefile                         |  1 +
  www/manager6/form/SpiceEnhancementSelector.js | 66 +++++++++++++++++++
  2 files changed, 67 insertions(+)
  create mode 100644 www/manager6/form/SpiceEnhancementSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 82e25c79..aa460c3b 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -66,6 +66,7 @@ JSSRC=                                                        
\
        form/CalendarEvent.js                           \
        form/CephPoolSelector.js                        \
        form/PermPathSelector.js                        \
+       form/SpiceEnhancementSelector.js                \
        dc/Tasks.js                                     \
        dc/Log.js                                       \
        panel/StatusPanel.js                            \
diff --git a/www/manager6/form/SpiceEnhancementSelector.js 
b/www/manager6/form/SpiceEnhancementSelector.js
new file mode 100644
index 00000000..5457c8d5
--- /dev/null
+++ b/www/manager6/form/SpiceEnhancementSelector.js
@@ -0,0 +1,66 @@
+Ext.define('PVE.form.SpiceEnhancementSelector', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: 'widget.pveSpiceEnhancementSelector',
+
+    initComponent: function() {
+       var me = this;
+       me.items= [
+           {
+               xtype: 'proxmoxcheckbox',
+               itemId: 'foldersharing',
+               name: 'foldersharing',
+               submitValue: false,
+               fieldLabel: gettext('Folder sharing'),
+               uncheckedValue: 0,
+           },
+           {
+               xtype: 'proxmoxKVComboBox',
+               itemId: 'videostreaming',
+               name: 'videostreaming',
+               submitValue: false,
+               value: 'off',
+               fieldLabel: gettext('Video streaming'),
+               comboItems: [
+                   ['off', 'off'],
+                   ['all', 'all'],
+                   ['filter', 'filter'],
+               ],
+           },
+       ];

Why not just set the items array on the object directly? Then you wouldn't have to override initComponent.

+       me.callParent();
+    },
+
+    // handle submitted values manually to work in the VM create wizard as 
well.
+    // without submitValue = false the fields would be added to the config
+    onGetValues: function() {
+       var me = this;
+
+       if (me.disabled) {
+           return

Missing ;

+       }
+
+       var values = {};
+       var foldersharing = me.down('field[name=foldersharing]').getValue();
+       var videostreaming= me.down('field[name=videostreaming]').getValue();

Missing space before =

+
+       if (videostreaming !== "off") {
+           values.videostreaming = videostreaming;
+       }
+       if (foldersharing) {
+           values.foldersharing = 1;
+       }
+       if (Ext.Object.isEmpty(values)) {
+           return { 'delete': 'spice_enhancements' };
+       }
+       var enhancements = PVE.Parser.printPropertyString(values);
+       return { spice_enhancements: enhancements };
+    },
+
+    setValues: function(values) {
+       var enhancements = values.spice_enhancements || '';
+       if (enhancements) {
+           var res = PVE.Parser.parsePropertyString(enhancements);

Same as in 3/4, foldersharing would need to be converted to a true boolean here (from potentially "on", "yes", ...).

+           this.callParent([res]);
+       }
+    },
+});


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to