9:22 a.m.
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We don not need a special "save" button for permission form.
All form in guest edit tab can share the same "save" button.
also, we will add a password form, it will also share this button.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 5 -----
ui/js/src/kimchi.guest_edit_main.js | 40 +++++++++++++++++++++----------------
ui/pages/guest-edit.html.tmpl | 3 ---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 1092cc9..74c2237 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -261,8 +261,3 @@
width: 46%;
float: right;
}
-
-#form-guest-edit-permission-save {
- float: right;
- margin-right: 10px;
-}
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 38a2bc0..7d24b44 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -19,13 +19,11 @@ kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
+ var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
$(buttonContainer).addClass('hidden');
- $("#form-guest-edit-permission-save").addClass('hidden');
var deactivated = ui['newPanel'];
- if($(deactivated).attr('id') === 'form-guest-edit-general')
{
+ if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
- }else if($(deactivated).attr('id') ===
'form-guest-edit-permission'){
-
$("#form-guest-edit-permission-save").removeClass('hidden');
}
}
});
@@ -335,18 +333,6 @@ kimchi.guest_edit_main = function() {
filterNodes("", $("#permission-avail-users"));
filterNodes("", $("#permission-avail-groups"));
});
- $("#form-guest-edit-permission-save").on("click",
function(){
- var content = { users: [], groups: [] };
- $("#permission-sel-users").children().each(function(){
- content.users.push($("label", this).text());
- });
- $("#permission-sel-groups").children().each(function(){
- content.groups.push($("label", this).text());
- });
- kimchi.updateVM(kimchi.selectedGuest, content, function(){
- kimchi.window.close();
- });
- });
};
var initContent = function(guest) {
@@ -393,7 +379,7 @@ kimchi.guest_edit_main = function() {
kimchi.retrieveVM(kimchi.selectedGuest, initContent);
- var submitForm = function(event) {
+ var generalSubmit = function(event) {
$(saveButton).prop('disabled', true);
var data=$('#form-guest-edit-general').serializeObject();
if(data['memory']!=undefined) {
@@ -410,7 +396,27 @@ kimchi.guest_edit_main = function() {
kimchi.message.error(err.responseJSON.reason);
$(saveButton).prop('disabled', false);
});
+ }
+ var permissionSubmit = function(event) {
+ var content = { users: [], groups: [] };
+ $("#permission-sel-users").children().each(function(){
+ content.users.push($("label", this).text());
+ });
+ $("#permission-sel-groups").children().each(function(){
+ content.groups.push($("label", this).text());
+ });
+ kimchi.updateVM(kimchi.selectedGuest, content, function(){
+ kimchi.window.close();
+ });
+ }
+
+ // tap map, "general": 0, "storage": 1, "interface": 2,
"permission": 3, "password": 4
+ var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submitForm = function(event) {
+ var current = $('#guest-edit-tabs').tabs( "option",
"active" );
+ var submitFun = submit_map[current];
+ submitFun && submitFun(event);
event.preventDefault();
};
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index 1c1d7d4..f24f7de 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -150,9 +150,6 @@
<span class="text">$_("Save")</span>
</button>
</div>
- <button id="form-guest-edit-permission-save" class="btn-normal
hidden">
- <span class="text">$_("Save")</span>
- </button>
</footer>
</div>
<script id="cdrom-row-tmpl" type="text/html">
--
1.9.3
9:48 a.m.
Wen Wang tells me, there is also a bug.
some users find there maybe two buttons then they test.
On 08/11/2014 10:22 PM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We don not need a special "save" button for permission form.
All form in guest edit tab can share the same "save" button.
also, we will add a password form, it will also share this button.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 5 -----
ui/js/src/kimchi.guest_edit_main.js | 40 +++++++++++++++++++++----------------
ui/pages/guest-edit.html.tmpl | 3 ---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 1092cc9..74c2237 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -261,8 +261,3 @@
width: 46%;
float: right;
}
-
-#form-guest-edit-permission-save {
- float: right;
- margin-right: 10px;
-}
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 38a2bc0..7d24b44 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -19,13 +19,11 @@ kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
+ var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
$(buttonContainer).addClass('hidden');
- $("#form-guest-edit-permission-save").addClass('hidden');
var deactivated = ui['newPanel'];
- if($(deactivated).attr('id') === 'form-guest-edit-general')
{
+ if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
- }else if($(deactivated).attr('id') ===
'form-guest-edit-permission'){
-
$("#form-guest-edit-permission-save").removeClass('hidden');
}
}
});
@@ -335,18 +333,6 @@ kimchi.guest_edit_main = function() {
filterNodes("", $("#permission-avail-users"));
filterNodes("", $("#permission-avail-groups"));
});
- $("#form-guest-edit-permission-save").on("click",
function(){
- var content = { users: [], groups: [] };
- $("#permission-sel-users").children().each(function(){
- content.users.push($("label", this).text());
- });
- $("#permission-sel-groups").children().each(function(){
- content.groups.push($("label", this).text());
- });
- kimchi.updateVM(kimchi.selectedGuest, content, function(){
- kimchi.window.close();
- });
- });
};
var initContent = function(guest) {
@@ -393,7 +379,7 @@ kimchi.guest_edit_main = function() {
kimchi.retrieveVM(kimchi.selectedGuest, initContent);
- var submitForm = function(event) {
+ var generalSubmit = function(event) {
$(saveButton).prop('disabled', true);
var data=$('#form-guest-edit-general').serializeObject();
if(data['memory']!=undefined) {
@@ -410,7 +396,27 @@ kimchi.guest_edit_main = function() {
kimchi.message.error(err.responseJSON.reason);
$(saveButton).prop('disabled', false);
});
+ }
+ var permissionSubmit = function(event) {
+ var content = { users: [], groups: [] };
+ $("#permission-sel-users").children().each(function(){
+ content.users.push($("label", this).text());
+ });
+ $("#permission-sel-groups").children().each(function(){
+ content.groups.push($("label", this).text());
+ });
+ kimchi.updateVM(kimchi.selectedGuest, content, function(){
+ kimchi.window.close();
+ });
+ }
+
+ // tap map, "general": 0, "storage": 1, "interface":
2, "permission": 3, "password": 4
+ var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submitForm = function(event) {
+ var current = $('#guest-edit-tabs').tabs( "option",
"active" );
+ var submitFun = submit_map[current];
+ submitFun && submitFun(event);
event.preventDefault();
};
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index 1c1d7d4..f24f7de 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -150,9 +150,6 @@
<span class="text">$_("Save")</span>
</button>
</div>
- <button id="form-guest-edit-permission-save" class="btn-normal
hidden">
- <span class="text">$_("Save")</span>
- </button>
</footer>
</div>
<script id="cdrom-row-tmpl" type="text/html">
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center
10:38 a.m.
New subject: [PATCH] UI: set ticket password
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
After set the password, only the one knows the password can access
the vm graphic.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 36 +++++++++++++++++++++++++++++++++++-
ui/js/src/kimchi.guest_edit_main.js | 32 +++++++++++++++++++++++++++++++-
ui/pages/guest-edit.html.tmpl | 15 +++++++++++++++
3 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 74c2237..eeea1cd 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -29,7 +29,8 @@
}
#form-guest-edit-general,
-#form-guest-edit-storage {
+#form-guest-edit-storage,
+#form-guest-edit-password {
padding: 1em;
}
@@ -67,6 +68,39 @@
width: 486px;
}
+#form-guest-edit-password .guest-edit-wrapper-controls {
+ width: 486px;
+ margin: auto;
+ text-align: center;
+}
+
+.div-guest-edit-password {
+ margin: auto;
+ text-align: center;
+ padding-bottom: 5px;
+ padding-top: 7px;
+}
+
+.guest-edit-wrapper-controls input[type="password"] {
+ font-size: 16px;
+ height: 38px;
+ background: #fff;
+ -webkit-border-radius: 5px;
+ border-radius: 5px;
+ box-shadow: 2px 2px 2px #eee inset;
+ border-top: 1px solid #bbb;
+ border-left: 1px solid #bbb;
+ padding-left: 10px;
+ width: 220px;
+ margin: auto;
+ text-align: left;
+}
+
+.err-mess {
+ color: #C85305;
+ text-align: center;
+}
+
.guest-edit-wrapper-controls input[type="text"] {
font-size: 16px;
height: 38px;
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 7d24b44..482fb7a 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -17,6 +17,8 @@
*/
kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
+ var password1 = $("#form-guest-edit-password input[name='pw1']");
+ var password2 = $("#form-guest-edit-password input[name='pw2']");
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
@@ -25,12 +27,24 @@ kimchi.guest_edit_main = function() {
if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
}
+ if($(deactivated).attr('id') ===
'form-guest-edit-password'){
+ password2.val().length &&
$(buttonContainer).removeClass('hidden');
+ }
}
});
var guestEditForm = $('#form-guest-edit-general');
var saveButton = $('#guest-edit-button-save');
+ password1.on("keyup", function(event) {
+ $("#div-guest-edit-password-mess").hide();
+ });
+
+ password2.on("keyup", function(event) {
+ $("#div-guest-edit-password-mess").hide();
+ $(buttonContainer).removeClass('hidden');
+ });
+
var refreshCDROMs = function() {
kimchi.listVMStorages({
vm: kimchi.selectedGuest
@@ -398,6 +412,22 @@ kimchi.guest_edit_main = function() {
});
}
+ var passwordSubmit = function(event) {
+ if (password1.val() != password2.val()){
+ $("#div-guest-edit-password-mess").show();
+ }
+ else {
+ var data = {'ticket': {'passwd': password1.val()}};
+ kimchi.updateVM(kimchi.selectedGuest, data, function() {
+ kimchi.listVmsAuto();
+ kimchi.window.close();
+ }, function(err) {
+ kimchi.message.error(err.responseJSON.reason);
+ $(saveButton).prop('disabled', false);
+ });
+ }
+ }
+
var permissionSubmit = function(event) {
var content = { users: [], groups: [] };
$("#permission-sel-users").children().each(function(){
@@ -412,7 +442,7 @@ kimchi.guest_edit_main = function() {
}
// tap map, "general": 0, "storage": 1, "interface": 2,
"permission": 3, "password": 4
- var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submit_map = {0: generalSubmit, 3:permissionSubmit, 4: passwordSubmit};
var submitForm = function(event) {
var current = $('#guest-edit-tabs').tabs( "option",
"active" );
var submitFun = submit_map[current];
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index f24f7de..22b5207 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -41,6 +41,9 @@
<li>
<a
href="#form-guest-edit-permission">$_("Permission")</a>
</li>
+ <li>
+ <a
href="#form-guest-edit-password">$_("Password")</a>
+ </li>
</ul>
<form id="form-guest-edit-general">
<fieldset class="guest-edit-fieldset">
@@ -142,6 +145,18 @@
</div>
</div>
</form>
+ <form id="form-guest-edit-password"
class="guest-edit-password">
+ <div class="guest-edit-wrapper-controls">
+ <div class="div-guest-edit-password">
$_("Password:")</div>
+ <input type="password" name="pw1"
autofocus>
+ <div class="div-guest-edit-password">
$_("Confirm Password:")</div>
+ <input type="password" name="pw2">
+ </div>
+ <div id="div-guest-edit-password-mess"
class="err-mess" style="display: none;">
+ $_("The above two passwords are not same, please re-enter
them!")
+ </div>
+ <div class="body"></div>
+ </form>
</div>
</div>
<footer>
--
1.9.3
2:11 p.m.
New subject: [PATCH] UI: set ticket password
On 11-08-2014 12:38, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
After set the password, only the one knows the password can access
the vm graphic.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
- A user can't tell what this password is about by using Kimchi. It just
displays "Password". Password for what? *I* know it's related to VNC
access because I read it here on your patch's description, but an end
user won't be able to know.
- Use a consistent form style to display the text fields. As we still
don't use a universal style, at least be consistent inside the same
dialog. The tab "General" has labels and text fields on the same line,
aligned to the left of the dialog. The tab "Password" has labels and
text fields on different lines, they're not centered or aligned at all
to the dialog.
- Use a consistent error message. Kimchi has a standard JavaScript
function call to display error messages (kimchi.message.error), let's
not use yet another way of displaying error messages.
- What's the purpose of the "Save" button initial state as invisible? If
it's to prevent the user to set an empty password, that's not happening.
When the user types something on both text fields, the button becomes
visible. Then if the user removes the text they typed, the button still
keeps visible. An empty password can be set now. And even if the user
doesn't type anything at all and just presses the Tab key twice, the
button also becomes visible. An empty password can also be set this way.
We still don't use a consistent input data handling across all dialogs
in Kimchi, but this approach doesn't validate the input data correctly.
7:15 p.m.
New subject: [PATCH] UI: set ticket password
I don't think a new tab is needed for it.
From my view, we can add just one new input box on "General" tab and
label it as "Console password" (as it works for VNC and Spice)
On 08/11/2014 12:38 PM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
After set the password, only the one knows the password can access
the vm graphic.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 36 +++++++++++++++++++++++++++++++++++-
ui/js/src/kimchi.guest_edit_main.js | 32 +++++++++++++++++++++++++++++++-
ui/pages/guest-edit.html.tmpl | 15 +++++++++++++++
3 files changed, 81 insertions(+), 2 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 74c2237..eeea1cd 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -29,7 +29,8 @@
}
#form-guest-edit-general,
-#form-guest-edit-storage {
+#form-guest-edit-storage,
+#form-guest-edit-password {
padding: 1em;
}
@@ -67,6 +68,39 @@
width: 486px;
}
+#form-guest-edit-password .guest-edit-wrapper-controls {
+ width: 486px;
+ margin: auto;
+ text-align: center;
+}
+
+.div-guest-edit-password {
+ margin: auto;
+ text-align: center;
+ padding-bottom: 5px;
+ padding-top: 7px;
+}
+
+.guest-edit-wrapper-controls input[type="password"] {
+ font-size: 16px;
+ height: 38px;
+ background: #fff;
+ -webkit-border-radius: 5px;
+ border-radius: 5px;
+ box-shadow: 2px 2px 2px #eee inset;
+ border-top: 1px solid #bbb;
+ border-left: 1px solid #bbb;
+ padding-left: 10px;
+ width: 220px;
+ margin: auto;
+ text-align: left;
+}
+
+.err-mess {
+ color: #C85305;
+ text-align: center;
+}
+
.guest-edit-wrapper-controls input[type="text"] {
font-size: 16px;
height: 38px;
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 7d24b44..482fb7a 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -17,6 +17,8 @@
*/
kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
+ var password1 = $("#form-guest-edit-password input[name='pw1']");
+ var password2 = $("#form-guest-edit-password input[name='pw2']");
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
@@ -25,12 +27,24 @@ kimchi.guest_edit_main = function() {
if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
}
+ if($(deactivated).attr('id') ===
'form-guest-edit-password'){
+ password2.val().length &&
$(buttonContainer).removeClass('hidden');
+ }
}
});
var guestEditForm = $('#form-guest-edit-general');
var saveButton = $('#guest-edit-button-save');
+ password1.on("keyup", function(event) {
+ $("#div-guest-edit-password-mess").hide();
+ });
+
+ password2.on("keyup", function(event) {
+ $("#div-guest-edit-password-mess").hide();
+ $(buttonContainer).removeClass('hidden');
+ });
+
var refreshCDROMs = function() {
kimchi.listVMStorages({
vm: kimchi.selectedGuest
@@ -398,6 +412,22 @@ kimchi.guest_edit_main = function() {
});
}
+ var passwordSubmit = function(event) {
+ if (password1.val() != password2.val()){
+ $("#div-guest-edit-password-mess").show();
+ }
+ else {
+ var data = {'ticket': {'passwd': password1.val()}};
+ kimchi.updateVM(kimchi.selectedGuest, data, function() {
+ kimchi.listVmsAuto();
+ kimchi.window.close();
+ }, function(err) {
+ kimchi.message.error(err.responseJSON.reason);
+ $(saveButton).prop('disabled', false);
+ });
+ }
+ }
+
var permissionSubmit = function(event) {
var content = { users: [], groups: [] };
$("#permission-sel-users").children().each(function(){
@@ -412,7 +442,7 @@ kimchi.guest_edit_main = function() {
}
// tap map, "general": 0, "storage": 1, "interface":
2, "permission": 3, "password": 4
- var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submit_map = {0: generalSubmit, 3:permissionSubmit, 4: passwordSubmit};
var submitForm = function(event) {
var current = $('#guest-edit-tabs').tabs( "option",
"active" );
var submitFun = submit_map[current];
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index f24f7de..22b5207 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -41,6 +41,9 @@
<li>
<a
href="#form-guest-edit-permission">$_("Permission")</a>
</li>
+ <li>
+ <a
href="#form-guest-edit-password">$_("Password")</a>
+ </li>
</ul>
<form id="form-guest-edit-general">
<fieldset class="guest-edit-fieldset">
@@ -142,6 +145,18 @@
</div>
</div>
</form>
+ <form id="form-guest-edit-password"
class="guest-edit-password">
+ <div class="guest-edit-wrapper-controls">
+ <div class="div-guest-edit-password">
$_("Password:")</div>
+ <input type="password" name="pw1"
autofocus>
+ <div class="div-guest-edit-password">
$_("Confirm Password:")</div>
+ <input type="password" name="pw2">
+ </div>
+ <div id="div-guest-edit-password-mess"
class="err-mess" style="display: none;">
+ $_("The above two passwords are not same, please re-enter
them!")
+ </div>
+ <div class="body"></div>
+ </form>
</div>
</div>
<footer>
1:41 p.m.
On 11-08-2014 11:22, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We don not need a special "save" button for permission form.
All form in guest edit tab can share the same "save" button.
also, we will add a password form, it will also share this button.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
Why are we using a save button for each tab in that dialog? That's
definitely not a common user experience. One would expect that a "Save"
button in that dialog would apply all changes done in the dialog, not on
that tab alone. What if the user wants to change the VM name and
permission (and the password, after the next patch)? They will need to
open the "Edit Guest" dialog, changing the value they want to change,
and then saving that value alone. /Three times./ That's not a good flow.
IMO, we should have only one "Save" button which, when clicked, applies
all the changes done in that dialog. Not a button which does different
things depending on which tab the user is on.
3:52 a.m.
On 08/12/2014 02:41 AM, Crístian Viana wrote:
On 11-08-2014 11:22, shaohef(a)linux.vnet.ibm.com wrote:
> From: ShaoHe Feng<shaohef(a)linux.vnet.ibm.com>
>
> We don not need a special "save" button for permission form.
>
> All form in guest edit tab can share the same "save" button.
>
> also, we will add a password form, it will also share this button.
>
> Signed-off-by: ShaoHe Feng<shaohef(a)linux.vnet.ibm.com>
> Signed-off-by: Simon Jin<simonjin(a)linux.vnet.ibm.com>
>
Why are we using a save button for each tab in that dialog? That's
definitely not a common user experience. One would expect that a
"Save" button in that dialog would apply all changes done in the
dialog, not on that tab alone. What if the user wants to change the VM
name and permission (and the password, after the next patch)? They
will need to open the "Edit Guest" dialog, changing the value they
want to change, and then saving that value alone. /Three times./
That's not a good flow.
IMO, we should have only one "Save" button which, when clicked,
applies all the changes done in that dialog. Not a button which does
different things depending on which tab the user is on.
That's not always true,
at least on virt-manager.
one thing can be improved here is to disable the "save" button when
there isn't having any change on the tab yet and enable it when there is.
-Simon
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
--
Yun Tong Jin, Simon
Linux Technology Center, Open Virtualization project
IBM Systems& Technology Group
jinyt(a)cn.ibm.com, Phone: 824549654
3:46 a.m.
On 8/12/2014 2:41 AM, Crístian Viana wrote:
On 11-08-2014 11:22, shaohef(a)linux.vnet.ibm.com wrote:
> From: ShaoHe Feng<shaohef(a)linux.vnet.ibm.com>
>
> We don not need a special "save" button for permission form.
>
> All form in guest edit tab can share the same "save" button.
>
> also, we will add a password form, it will also share this button.
>
> Signed-off-by: ShaoHe Feng<shaohef(a)linux.vnet.ibm.com>
> Signed-off-by: Simon Jin<simonjin(a)linux.vnet.ibm.com>
>
Why are we using a save button for each tab in that dialog? That's
definitely not a common user experience. One would expect that a
"Save" button in that dialog would apply all changes done in the
dialog, not on that tab alone. What if the user wants to change the VM
name and permission (and the password, after the next patch)? They
will need to open the "Edit Guest" dialog, changing the value they
want to change, and then saving that value alone. /Three times./
That's not a good flow.
IMO, we should have only one "Save" button which, when clicked,
applies all the changes done in that dialog. Not a button which does
different things depending on which tab the user is on.
1. Right now, there are already many options when editing VM, and I
believe there will be more and more.
Each time user edit VM, they will only edit one or two options(a
small portion of all), save all the options back is inefficient and go
against user's intension.
2. The design need to be changed to save what user has changed and allow
user to change at a fine granularity, I noticed that virt-manager did a
good job here.
In virt-manager, user can update anything in a quite fine
granularity and the 'apply' button feels quite make sense to indicate
that it just update what user is changing.
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
7:10 p.m.
Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Tested-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
On 08/11/2014 11:22 AM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We don not need a special "save" button for permission form.
All form in guest edit tab can share the same "save" button.
also, we will add a password form, it will also share this button.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 5 -----
ui/js/src/kimchi.guest_edit_main.js | 40 +++++++++++++++++++++----------------
ui/pages/guest-edit.html.tmpl | 3 ---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 1092cc9..74c2237 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -261,8 +261,3 @@
width: 46%;
float: right;
}
-
-#form-guest-edit-permission-save {
- float: right;
- margin-right: 10px;
-}
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 38a2bc0..7d24b44 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -19,13 +19,11 @@ kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
+ var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
$(buttonContainer).addClass('hidden');
- $("#form-guest-edit-permission-save").addClass('hidden');
var deactivated = ui['newPanel'];
- if($(deactivated).attr('id') === 'form-guest-edit-general')
{
+ if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
- }else if($(deactivated).attr('id') ===
'form-guest-edit-permission'){
-
$("#form-guest-edit-permission-save").removeClass('hidden');
}
}
});
@@ -335,18 +333,6 @@ kimchi.guest_edit_main = function() {
filterNodes("", $("#permission-avail-users"));
filterNodes("", $("#permission-avail-groups"));
});
- $("#form-guest-edit-permission-save").on("click",
function(){
- var content = { users: [], groups: [] };
- $("#permission-sel-users").children().each(function(){
- content.users.push($("label", this).text());
- });
- $("#permission-sel-groups").children().each(function(){
- content.groups.push($("label", this).text());
- });
- kimchi.updateVM(kimchi.selectedGuest, content, function(){
- kimchi.window.close();
- });
- });
};
var initContent = function(guest) {
@@ -393,7 +379,7 @@ kimchi.guest_edit_main = function() {
kimchi.retrieveVM(kimchi.selectedGuest, initContent);
- var submitForm = function(event) {
+ var generalSubmit = function(event) {
$(saveButton).prop('disabled', true);
var data=$('#form-guest-edit-general').serializeObject();
if(data['memory']!=undefined) {
@@ -410,7 +396,27 @@ kimchi.guest_edit_main = function() {
kimchi.message.error(err.responseJSON.reason);
$(saveButton).prop('disabled', false);
});
+ }
+ var permissionSubmit = function(event) {
+ var content = { users: [], groups: [] };
+ $("#permission-sel-users").children().each(function(){
+ content.users.push($("label", this).text());
+ });
+ $("#permission-sel-groups").children().each(function(){
+ content.groups.push($("label", this).text());
+ });
+ kimchi.updateVM(kimchi.selectedGuest, content, function(){
+ kimchi.window.close();
+ });
+ }
+
+ // tap map, "general": 0, "storage": 1, "interface":
2, "permission": 3, "password": 4
+ var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submitForm = function(event) {
+ var current = $('#guest-edit-tabs').tabs( "option",
"active" );
+ var submitFun = submit_map[current];
+ submitFun && submitFun(event);
event.preventDefault();
};
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index 1c1d7d4..f24f7de 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -150,9 +150,6 @@
<span class="text">$_("Save")</span>
</button>
</div>
- <button id="form-guest-edit-permission-save" class="btn-normal
hidden">
- <span class="text">$_("Save")</span>
- </button>
</footer>
</div>
<script id="cdrom-row-tmpl" type="text/html">
8:28 p.m.
I will apply just this patch as it is already reviewed and tested and
does not depends on the other one
On 08/11/2014 11:22 AM, shaohef(a)linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
We don not need a special "save" button for permission form.
All form in guest edit tab can share the same "save" button.
also, we will add a password form, it will also share this button.
Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Signed-off-by: Simon Jin <simonjin(a)linux.vnet.ibm.com>
---
ui/css/theme-default/guest-edit.css | 5 -----
ui/js/src/kimchi.guest_edit_main.js | 40 +++++++++++++++++++++----------------
ui/pages/guest-edit.html.tmpl | 3 ---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/ui/css/theme-default/guest-edit.css b/ui/css/theme-default/guest-edit.css
index 1092cc9..74c2237 100644
--- a/ui/css/theme-default/guest-edit.css
+++ b/ui/css/theme-default/guest-edit.css
@@ -261,8 +261,3 @@
width: 46%;
float: right;
}
-
-#form-guest-edit-permission-save {
- float: right;
- margin-right: 10px;
-}
diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js
index 38a2bc0..7d24b44 100644
--- a/ui/js/src/kimchi.guest_edit_main.js
+++ b/ui/js/src/kimchi.guest_edit_main.js
@@ -19,13 +19,11 @@ kimchi.guest_edit_main = function() {
var buttonContainer = $('#action-button-container');
$('#guest-edit-tabs').tabs({
beforeActivate: function(event, ui) {
+ var display_list = ['form-guest-edit-general',
'form-guest-edit-permission']
$(buttonContainer).addClass('hidden');
- $("#form-guest-edit-permission-save").addClass('hidden');
var deactivated = ui['newPanel'];
- if($(deactivated).attr('id') === 'form-guest-edit-general')
{
+ if(display_list.indexOf($(deactivated).attr('id')) >= 0) {
$(buttonContainer).removeClass('hidden');
- }else if($(deactivated).attr('id') ===
'form-guest-edit-permission'){
-
$("#form-guest-edit-permission-save").removeClass('hidden');
}
}
});
@@ -335,18 +333,6 @@ kimchi.guest_edit_main = function() {
filterNodes("", $("#permission-avail-users"));
filterNodes("", $("#permission-avail-groups"));
});
- $("#form-guest-edit-permission-save").on("click",
function(){
- var content = { users: [], groups: [] };
- $("#permission-sel-users").children().each(function(){
- content.users.push($("label", this).text());
- });
- $("#permission-sel-groups").children().each(function(){
- content.groups.push($("label", this).text());
- });
- kimchi.updateVM(kimchi.selectedGuest, content, function(){
- kimchi.window.close();
- });
- });
};
var initContent = function(guest) {
@@ -393,7 +379,7 @@ kimchi.guest_edit_main = function() {
kimchi.retrieveVM(kimchi.selectedGuest, initContent);
- var submitForm = function(event) {
+ var generalSubmit = function(event) {
$(saveButton).prop('disabled', true);
var data=$('#form-guest-edit-general').serializeObject();
if(data['memory']!=undefined) {
@@ -410,7 +396,27 @@ kimchi.guest_edit_main = function() {
kimchi.message.error(err.responseJSON.reason);
$(saveButton).prop('disabled', false);
});
+ }
+ var permissionSubmit = function(event) {
+ var content = { users: [], groups: [] };
+ $("#permission-sel-users").children().each(function(){
+ content.users.push($("label", this).text());
+ });
+ $("#permission-sel-groups").children().each(function(){
+ content.groups.push($("label", this).text());
+ });
+ kimchi.updateVM(kimchi.selectedGuest, content, function(){
+ kimchi.window.close();
+ });
+ }
+
+ // tap map, "general": 0, "storage": 1, "interface":
2, "permission": 3, "password": 4
+ var submit_map = {0: generalSubmit, 3:permissionSubmit};
+ var submitForm = function(event) {
+ var current = $('#guest-edit-tabs').tabs( "option",
"active" );
+ var submitFun = submit_map[current];
+ submitFun && submitFun(event);
event.preventDefault();
};
diff --git a/ui/pages/guest-edit.html.tmpl b/ui/pages/guest-edit.html.tmpl
index 1c1d7d4..f24f7de 100644
--- a/ui/pages/guest-edit.html.tmpl
+++ b/ui/pages/guest-edit.html.tmpl
@@ -150,9 +150,6 @@
<span class="text">$_("Save")</span>
</button>
</div>
- <button id="form-guest-edit-permission-save" class="btn-normal
hidden">
- <span class="text">$_("Save")</span>
- </button>
</footer>
</div>
<script id="cdrom-row-tmpl" type="text/html">
3783
days inactive
3786
days old
10 comments
6 participants
participants (6)
-
Aline Manera -
Crístian Viana -
shaohef＠linux.vnet.ibm.com -
Sheldon
-
simonjin -
Yu Xin Huo