Re: [Kimchi-devel] [PATCH] UI: Guest Snapshot

Here are my comments on this patch: - When I open the dialog "Edit" on a VM without snapshots, the error KCHSNAP0007E* is shown. That's because the VM doesn't have snapshots but the UI should handle that and do nothing. Not having snapshots is not an error from the UI perspective. - I cannot create a snapshot at all. Whenever I click on the button '+', the error "KCHAPI0006E: Unable to parse JSON request" is shown. - I cannot revert to the current snapshot. The button "Revert" becomes disabled whenever the current snapshot is selected. And that's a very important use case. Suppose I just created a snapshot. Then I mistakenly removed some files on my guest system and I want to revert to that snapshot. With the current UI, I'm not able to do that. - There's no visual feedback when I click on the button "Revert". When I click on it and the revert operation starts, nothing happens. The user might click on it again thinking that the button wasn't clicked on the first time. Also there's no way to know if the revert operation finished. - If the user clicks on the button "Revert", closes the dialog "Edit" and reopens it while the revert operation is still happening, the error KCHSNAP0007E* is shown and the tab "Snapshot" is rendered empty, even if there are snapshots to be listed on the VM. The same problem also happens when the user clicks on the button with the trash can, instead of "Revert", and closes the dialog and reopens it while the operation hasn't finished. - Why isn't there a button "Revert" on each snapshot line? The button with the trash can is there, the button "Revert" should also be there. That's an action that applies to only one snapshot at a time, just like delete. Other actions in other tabs, like "Edit", are also shown on the relevant line instead of at the top of the dialog. The button "+" is different, though, because it doesn't act in one entry alone, it acts on the whole set of items. So that makes sense for me to be at the top. * Actually, the backend is raising the exception code KCHSNAP007E (i.e. there's one '0' less), which is an invalid code. I'll send a patch later to fix that. But the UI shouldn't worry about that because the REST command returns the status code 404 in that case anyway. On 17-11-2014 09:12, huoyuxin(a)linux.vnet.ibm.com wrote: