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

[Rodrigo Trujillo]

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 at 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 at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>