Re: [Kimchi-devel] [PATCH] Fix Add Network window

26 Feb 2014

Thanks Mark, makes the code more understandable  =]
However, you missed one case:
- Suppose you click on Bridge, and you have more than 1 interface
- You select any other interface than "0", click on Vlan and fill a Vlan ID
- If you click in Bridged option again, the interface selection will 
move to "0" (vlan fields remain unchanged).

This may lead to an error.

I am sending a new version of the patch.

Rodrigo



On 02/26/2014 09:06 AM, Mark Wu wrote:
...
On 02/26/2014 03:17 AM, Rodrigo Trujillo wrote:
...
VLans and Interfaces must only be available for selection if network
type Bridge is selected. This restriction was not implemented in the UI.
This patch fixes this problem.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com>
---
  ui/js/src/kimchi.network.js | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
index faf1285..4c25ad0 100644
--- a/ui/js/src/kimchi.network.js
+++ b/ui/js/src/kimchi.network.js
@@ -210,11 +210,33 @@ kimchi.openNetworkDialog = function(okCallback) {
      $("#networkConfig").dialog("open");
  };
+kimchi.enableBridgeOptions = function(enable) {
+    var ifaces = 
document.getElementById("networkInterface").getElementsByTagName("option");
+    for (var i = 0; i < ifaces.length; i++) {
+        ifaces[i].disabled = (!enable);
+        if (!enable) {
+            ifaces[i].selected = false;
+        }
+        else if (!$("#networkInterface").val()) {
+            ifaces[0].selected = true;
+        }
+    }
+    $("#enableVlan").prop("disabled", !enable);
+    if (!$("#networkTypeBri").prop("checked")) {
+        $("#enableVlan").attr("checked", false);
+        $("#networkVlanID").prop("disabled", true);
+        $("#networkVlanID").val("");
+    }
+};
+
  kimchi.setDefaultNetworkType = function(isInterfaceAvail) {
      $("#networkTypeBri").prop("checked", isInterfaceAvail);
      $("#networkTypeBri").prop("disabled", !isInterfaceAvail);
      $("#networkInterface").prop("disabled", !isInterfaceAvail);
      $("#networkTypeNat").prop("checked", !isInterfaceAvail);
+    if (!isInterfaceAvail) {
+        kimchi.enableBridgeOptions(false);
+    }
  };
kimchi.getNetworkDialogValues = function() {
@@ -250,12 +272,15 @@ kimchi.setupNetworkFormEvent = function() {
      });
      $("#networkTypeIso").on("click", function(event) {
          $("#networkInterface").prop("disabled", true);
+        kimchi.enableBridgeOptions(false);
      });
      $("#networkTypeNat").on("click", function(event) {
          $("#networkInterface").prop("disabled", true);
+        kimchi.enableBridgeOptions(false);
      });
      $("#networkTypeBri").on("click", function(event) {
          $("#networkInterface").prop("disabled", false);
+        kimchi.enableBridgeOptions(true);
      });
  };
I don't have much knowledge about js.  But it seems we can improve it 
a little bit.
The following change based your patch works for me.
diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
index 4c25ad0..467050c 100644
--- a/ui/js/src/kimchi.network.js
+++ b/ui/js/src/kimchi.network.js
@@ -211,28 +211,24 @@ kimchi.openNetworkDialog = function(okCallback) {
 };
kimchi.enableBridgeOptions = function(enable) {
-    var ifaces = 
document.getElementById("networkInterface").getElementsByTagName("option");
-    for (var i = 0; i < ifaces.length; i++) {
-        ifaces[i].disabled = (!enable);
-        if (!enable) {
-            ifaces[i].selected = false;
-        }
-        else if (!$("#networkInterface").val()) {
-            ifaces[0].selected = true;
-        }
-    }
     $("#enableVlan").prop("disabled", !enable);
-    if (!$("#networkTypeBri").prop("checked")) {
+    $("#networkInterface").prop("disabled", !enable);
+
+    if (!enable) {
         $("#enableVlan").attr("checked", false);
         $("#networkVlanID").prop("disabled", true);
         $("#networkVlanID").val("");
+        $("#networkInterface").val("");
+    } else {
+        $("#networkInterface").prop("selectedIndex", 0);
     }
+
+
 };
kimchi.setDefaultNetworkType = function(isInterfaceAvail) {
     $("#networkTypeBri").prop("checked", isInterfaceAvail);
     $("#networkTypeBri").prop("disabled", !isInterfaceAvail);
-    $("#networkInterface").prop("disabled", !isInterfaceAvail);
     $("#networkTypeNat").prop("checked", !isInterfaceAvail);
     if (!isInterfaceAvail) {
         kimchi.enableBridgeOptions(false);
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel


    