Re: [pve-devel] [PATCH manager 2/4] spice: Add enhancements form component

[Aaron Lauterer]

On 9/16/19 2:44 PM, Stefan Reiter wrote:

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 ;

Thx for seeing this

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

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

Missing space before =

Thx

+
+    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", ...).

Good point.

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

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