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

[Crístian Viana]

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 at linux.vnet.ibm.com wrote:
> From: Yu Xin Huo <huoyuxin at linux.vnet.ibm.com>
>
> Signed-off-by: Yu Xin Huo <huoyuxin at linux.vnet.ibm.com>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20141117/6bd3fbc9/attachment.html>