[PATCH] UI: refactor guest edit code.
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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
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@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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@linux.vnet.ibm.com> IBM Linux Technology Center
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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
On 11-08-2014 12:38, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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.
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@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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>
On 11-08-2014 11:22, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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.
On 11-08-2014 11:22, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng<shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin<simonjin@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
On 08/12/2014 02:41 AM, Crístian Viana wrote: there isn't having any change on the tab yet and enable it when there is. -Simon
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@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@cn.ibm.com, Phone: 824549654
On 8/12/2014 2:41 AM, Crístian Viana wrote:
On 11-08-2014 11:22, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng<shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin<simonjin@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> Tested-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 08/11/2014 11:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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">
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@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Simon Jin <simonjin@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">
participants (6)
-
Aline Manera -
Crístian Viana -
shaohef＠linux.vnet.ibm.com -
Sheldon
-
simonjin -
Yu Xin Huo