4:57 p.m.
From: Aline Manera <alinefm(a)br.ibm.com>
V1 -> V2:
- Remove alert used for testing
Aline Manera (1):
UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
ui/js/src/kimchi.network.js | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--
1.7.10.4
4:57 p.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
From: Aline Manera <alinefm(a)br.ibm.com>
Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
while creating a bridged network, but it doesn't check the checkbox before
setting the vlan_id to send server request.
Because that when the user tries to create a bridged network without enabling
VLAN the following error is raised:
Invalid parameter: 'None is not of type u'integer''
Fix it by setting vlan_id only if the vlan checkbox is checked.
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
ui/js/src/kimchi.network.js | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
index 65479b2..eb64e03 100644
--- a/ui/js/src/kimchi.network.js
+++ b/ui/js/src/kimchi.network.js
@@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
var data = {
name : network.name,
connection: network.type,
- vlan_id: network.vlan_id,
};
if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
data.connection = "bridge";
data.interface = network.interface;
- data.vlan_id = network.vlan_id;
+ if ($("#enableVlan").prop("checked")) {
+ data.vlan_id = network.vlan_id;
+ }
}
kimchi.createNetwork(data, function(result) {
network.state = result.state === "active" ? "up" :
"down";
--
1.7.10.4
3:04 a.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
the vlan id need to be validated, it must be numeric and within 1 <=
vlan id <= 4094.
I dislike the UI design, please change it.
On 1/15/2014 7:57 AM, Aline Manera wrote:
From: Aline Manera <alinefm(a)br.ibm.com>
Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
while creating a bridged network, but it doesn't check the checkbox before
setting the vlan_id to send server request.
Because that when the user tries to create a bridged network without enabling
VLAN the following error is raised:
Invalid parameter: 'None is not of type u'integer''
Fix it by setting vlan_id only if the vlan checkbox is checked.
Signed-off-by: Aline Manera <alinefm(a)br.ibm.com>
---
ui/js/src/kimchi.network.js | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
index 65479b2..eb64e03 100644
--- a/ui/js/src/kimchi.network.js
+++ b/ui/js/src/kimchi.network.js
@@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
var data = {
name : network.name,
connection: network.type,
- vlan_id: network.vlan_id,
};
if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
data.connection = "bridge";
data.interface = network.interface;
- data.vlan_id = network.vlan_id;
+ if ($("#enableVlan").prop("checked")) {
+ data.vlan_id = network.vlan_id;
+ }
}
kimchi.createNetwork(data, function(result) {
network.state = result.state === "active" ? "up" :
"down";
4:26 a.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
On 01/15/2014 08:04 AM, Yu Xin Huo wrote:
the vlan id need to be validated, it must be numeric and within 1
<=
vlan id <= 4094.
ACK. I will add it in V3.
I dislike the UI design, please change it.
This specific patch is to fix bug instead of redesign.
So please, send a patch to redesign this view.
I also would like to suggest more improvements to this network creation
window.
1. To select a network type I need to select the circle itself, which is
boring.
In other views selecting the text select the item. You need to do it
here too.
2. The additional input boxes for bridged network should only appear
when selecting this option.
So in a first moment, we only show the 2 options.
When selecting "Briged" option we display:
Destination: | <iface> |
* Enable VLAN
When only when selecting "Enable VLAN" we display the vlan_id input
box.
On 1/15/2014 7:57 AM, Aline Manera wrote:
> From: Aline Manera<alinefm(a)br.ibm.com>
>
> Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
> while creating a bridged network, but it doesn't check the checkbox before
> setting the vlan_id to send server request.
>
> Because that when the user tries to create a bridged network without enabling
> VLAN the following error is raised:
> Invalid parameter: 'None is not of type u'integer''
>
> Fix it by setting vlan_id only if the vlan checkbox is checked.
>
> Signed-off-by: Aline Manera<alinefm(a)br.ibm.com>
> ---
> ui/js/src/kimchi.network.js | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
> index 65479b2..eb64e03 100644
> --- a/ui/js/src/kimchi.network.js
> +++ b/ui/js/src/kimchi.network.js
> @@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
> var data = {
> name : network.name,
> connection: network.type,
> - vlan_id: network.vlan_id,
> };
> if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
> data.connection = "bridge";
> data.interface = network.interface;
> - data.vlan_id = network.vlan_id;
> + if ($("#enableVlan").prop("checked")) {
> + data.vlan_id = network.vlan_id;
> + }
> }
> kimchi.createNetwork(data, function(result) {
> network.state = result.state === "active" ?
"up" : "down";
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
6:46 p.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
On 01/15/2014 07:26 PM, Aline Manera wrote:
On 01/15/2014 08:04 AM, Yu Xin Huo wrote:
> the vlan id need to be validated, it must be numeric and within 1 <=
> vlan id <= 4094.
ACK. I will add it in V3.
Could you please also consider to include the following fix:
$("#enableVlan").on("click", function() {
$("#networkVlanID").prop("disabled", !this.checked);
+ if (!this.checked) {
+ $("#networkVlanID").val("");
+ }
to reset the text when the checkbox is unchecked.
It looks that I should not touch UI code any more.
>
> I dislike the UI design, please change it.
This specific patch is to fix bug instead of redesign.
So please, send a patch to redesign this view.
I also would like to suggest more improvements to this network
creation window.
1. To select a network type I need to select the circle itself, which
is boring.
In other views selecting the text select the item. You need to do
it here too.
2. The additional input boxes for bridged network should only appear
when selecting this option.
So in a first moment, we only show the 2 options.
When selecting "Briged" option we display:
Destination: | <iface> |
* Enable VLAN
When only when selecting "Enable VLAN" we display the vlan_id
input box.
>
>
>
> On 1/15/2014 7:57 AM, Aline Manera wrote:
>> From: Aline Manera<alinefm(a)br.ibm.com>
>>
>> Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
>> while creating a bridged network, but it doesn't check the checkbox before
>> setting the vlan_id to send server request.
>>
>> Because that when the user tries to create a bridged network without enabling
>> VLAN the following error is raised:
>> Invalid parameter: 'None is not of type u'integer''
>>
>> Fix it by setting vlan_id only if the vlan checkbox is checked.
>>
>> Signed-off-by: Aline Manera<alinefm(a)br.ibm.com>
>> ---
>> ui/js/src/kimchi.network.js | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
>> index 65479b2..eb64e03 100644
>> --- a/ui/js/src/kimchi.network.js
>> +++ b/ui/js/src/kimchi.network.js
>> @@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
>> var data = {
>> name : network.name,
>> connection: network.type,
>> - vlan_id: network.vlan_id,
>> };
>> if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
>> data.connection = "bridge";
>> data.interface = network.interface;
>> - data.vlan_id = network.vlan_id;
>> + if ($("#enableVlan").prop("checked")) {
>> + data.vlan_id = network.vlan_id;
>> + }
>> }
>> kimchi.createNetwork(data, function(result) {
>> network.state = result.state === "active" ?
"up" : "down";
>
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
11:01 p.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
On 1/16/2014 9:46 AM, Mark Wu wrote:
On 01/15/2014 07:26 PM, Aline Manera wrote:
> On 01/15/2014 08:04 AM, Yu Xin Huo wrote:
>> the vlan id need to be validated, it must be numeric and within 1 <=
>> vlan id <= 4094.
>
> ACK. I will add it in V3.
Could you please also consider to include the following fix:
$("#enableVlan").on("click", function() {
$("#networkVlanID").prop("disabled", !this.checked);
+ if (!this.checked) {
+ $("#networkVlanID").val("");
+ }
to reset the text when the checkbox is unchecked.
It looks that I should not touch UI code any more.
Please feel free to touch UI
code, I strongly support mutual learning
between frontend and backend. If any difficulty, I would like to help.
>
>>
>> I dislike the UI design, please change it.
>
> This specific patch is to fix bug instead of redesign.
> So please, send a patch to redesign this view.
>
> I also would like to suggest more improvements to this network
> creation window.
>
> 1. To select a network type I need to select the circle itself, which
> is boring.
> In other views selecting the text select the item. You need to do
> it here too.
>
> 2. The additional input boxes for bridged network should only appear
> when selecting this option.
> So in a first moment, we only show the 2 options.
> When selecting "Briged" option we display:
>
> Destination: | <iface> |
> * Enable VLAN
>
> When only when selecting "Enable VLAN" we display the vlan_id
> input box.
>
>>
>>
>>
>> On 1/15/2014 7:57 AM, Aline Manera wrote:
>>> From: Aline Manera<alinefm(a)br.ibm.com>
>>>
>>> Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
>>> while creating a bridged network, but it doesn't check the checkbox
before
>>> setting the vlan_id to send server request.
>>>
>>> Because that when the user tries to create a bridged network without
enabling
>>> VLAN the following error is raised:
>>> Invalid parameter: 'None is not of type u'integer''
>>>
>>> Fix it by setting vlan_id only if the vlan checkbox is checked.
>>>
>>> Signed-off-by: Aline Manera<alinefm(a)br.ibm.com>
>>> ---
>>> ui/js/src/kimchi.network.js | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
>>> index 65479b2..eb64e03 100644
>>> --- a/ui/js/src/kimchi.network.js
>>> +++ b/ui/js/src/kimchi.network.js
>>> @@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
>>> var data = {
>>> name : network.name,
>>> connection: network.type,
>>> - vlan_id: network.vlan_id,
>>> };
>>> if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
>>> data.connection = "bridge";
>>> data.interface = network.interface;
>>> - data.vlan_id = network.vlan_id;
>>> + if ($("#enableVlan").prop("checked")) {
>>> + data.vlan_id = network.vlan_id;
>>> + }
>>> }
>>> kimchi.createNetwork(data, function(result) {
>>> network.state = result.state === "active" ?
"up" : "down";
>>
>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
5:16 a.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
On 01/15/2014 11:46 PM, Mark Wu wrote:
On 01/15/2014 07:26 PM, Aline Manera wrote:
> On 01/15/2014 08:04 AM, Yu Xin Huo wrote:
>> the vlan id need to be validated, it must be numeric and within 1 <=
>> vlan id <= 4094.
>
> ACK. I will add it in V3.
Could you please also consider to include the following fix:
$("#enableVlan").on("click", function() {
$("#networkVlanID").prop("disabled", !this.checked);
+ if (!this.checked) {
+ $("#networkVlanID").val("");
+ }
to reset the text when the checkbox is unchecked.
ACK. It will be in V2
It looks that I should not touch UI code any more.
>
>>
>> I dislike the UI design, please change it.
>
> This specific patch is to fix bug instead of redesign.
> So please, send a patch to redesign this view.
>
> I also would like to suggest more improvements to this network
> creation window.
>
> 1. To select a network type I need to select the circle itself, which
> is boring.
> In other views selecting the text select the item. You need to do
> it here too.
>
> 2. The additional input boxes for bridged network should only appear
> when selecting this option.
> So in a first moment, we only show the 2 options.
> When selecting "Briged" option we display:
>
> Destination: | <iface> |
> * Enable VLAN
>
> When only when selecting "Enable VLAN" we display the vlan_id
> input box.
>
>>
>>
>>
>> On 1/15/2014 7:57 AM, Aline Manera wrote:
>>> From: Aline Manera<alinefm(a)br.ibm.com>
>>>
>>> Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
>>> while creating a bridged network, but it doesn't check the checkbox
before
>>> setting the vlan_id to send server request.
>>>
>>> Because that when the user tries to create a bridged network without
enabling
>>> VLAN the following error is raised:
>>> Invalid parameter: 'None is not of type u'integer''
>>>
>>> Fix it by setting vlan_id only if the vlan checkbox is checked.
>>>
>>> Signed-off-by: Aline Manera<alinefm(a)br.ibm.com>
>>> ---
>>> ui/js/src/kimchi.network.js | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
>>> index 65479b2..eb64e03 100644
>>> --- a/ui/js/src/kimchi.network.js
>>> +++ b/ui/js/src/kimchi.network.js
>>> @@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
>>> var data = {
>>> name : network.name,
>>> connection: network.type,
>>> - vlan_id: network.vlan_id,
>>> };
>>> if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
>>> data.connection = "bridge";
>>> data.interface = network.interface;
>>> - data.vlan_id = network.vlan_id;
>>> + if ($("#enableVlan").prop("checked")) {
>>> + data.vlan_id = network.vlan_id;
>>> + }
>>> }
>>> kimchi.createNetwork(data, function(result) {
>>> network.state = result.state === "active" ?
"up" : "down";
>>
>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel(a)ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
11:20 p.m.
New subject: [PATCH] UI bug fix: Set vlan_id value only if 'Enable VLAN' is checked
On 1/15/2014 7:26 PM, Aline Manera wrote:
On 01/15/2014 08:04 AM, Yu Xin Huo wrote:
> the vlan id need to be validated, it must be numeric and within 1 <=
> vlan id <= 4094.
ACK. I will add it in V3.
>
> I dislike the UI design, please change it.
This specific patch is to fix bug instead of redesign.
So please, send a patch to redesign this view.
Opened an issue to track it.
https://github.com/kimchi-project/kimchi/issues/305
I also would like to suggest more improvements to this network
creation window.
1. To select a network type I need to select the circle itself, which
is boring.
In other views selecting the text select the item. You need to do
it here too.
Accept, quite easy, hope you can also get it handled in your V3.
2. The additional input boxes for bridged network should only appear
when selecting this option.
So in a first moment, we only show the 2 options.
When selecting "Briged" option we display:
Destination: | <iface> |
* Enable VLAN
When only when selecting "Enable VLAN" we display the vlan_id
input box.
From a usability perspective, I disagree with this point.
1. The space is not limited. We should disable something instead of
hide/show something per user selections.
2. To get everything there, user can totally figure out what to input
with consideration of all items. It greatly promote predictable UI.
3. If by default hide something and show it when user select something,
it will definitely delay user input which is inefficient design.
>
>
>
> On 1/15/2014 7:57 AM, Aline Manera wrote:
>> From: Aline Manera<alinefm(a)br.ibm.com>
>>
>> Commit 2f54aaba added a checkbox to provide choice of enabling vlan tagging
>> while creating a bridged network, but it doesn't check the checkbox before
>> setting the vlan_id to send server request.
>>
>> Because that when the user tries to create a bridged network without enabling
>> VLAN the following error is raised:
>> Invalid parameter: 'None is not of type u'integer''
>>
>> Fix it by setting vlan_id only if the vlan checkbox is checked.
>>
>> Signed-off-by: Aline Manera<alinefm(a)br.ibm.com>
>> ---
>> ui/js/src/kimchi.network.js | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js
>> index 65479b2..eb64e03 100644
>> --- a/ui/js/src/kimchi.network.js
>> +++ b/ui/js/src/kimchi.network.js
>> @@ -136,12 +136,13 @@ kimchi.initNetworkCreation = function() {
>> var data = {
>> name : network.name,
>> connection: network.type,
>> - vlan_id: network.vlan_id,
>> };
>> if (network.type === kimchi.NETWORK_TYPE_BRIDGE) {
>> data.connection = "bridge";
>> data.interface = network.interface;
>> - data.vlan_id = network.vlan_id;
>> + if ($("#enableVlan").prop("checked")) {
>> + data.vlan_id = network.vlan_id;
>> + }
>> }
>> kimchi.createNetwork(data, function(result) {
>> network.state = result.state === "active" ?
"up" : "down";
>
>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
3763
days inactive
3766
days old
7 comments
3 participants
participants (3)
-
Aline Manera -
Mark Wu -
Yu Xin Huo