[et-mgmt-tools] [PATCH] UI String Cleanups

[Cole Robinson]

Richard Laager wrote:
> On Mon, 2009-01-12 at 15:08 -0500, Cole Robinson wrote:
>> This is the correct mailing list, but it's probably better to send
>> things as 'hg exports' rather than bundles. No harm now though, but I'll
>> update the README.
> 
> This should probably be updated as well:
> http://virt-manager.et.redhat.com/scmrepo.html
> 
>>> This corrects a number of small UI issues. This should partially or
>>> fully address bugs 452411, 475682, and 475926.
> 
> I took a look at those (and other bugs we filed) and prepared a few more
> minor UI changes:
> 
> The attached patch should address the remainder of the issues in
> #452411, so it can be closed (assuming you agree with my comment about
> the pause action there).

Excellent, thanks.

> 
> It also addresses the wording in #475926 (and similar other cases), but
> does not address the buttons. I imagine I'd have to create the dialog
> myself, rather than use gtk.MessageDialog? I'm familiar with GTK+, but
> not the PyGTK wrapper, so any pointers would be helpful.
> 

This can hold off a bit. We need a revamped delete dialog anyways to
optionally allow deleting the VM disk storage, so I'll incorporate this
change when that work is done.

> The patch addresses the wording changes in #478408, but not the more
> in-depth suggestions:
>         Alternatively, if the first item in the list is always selected
>         (making it impossible to select nothing if the list is
>         non-empty), then the physical device radio button could be
>         greyed out when the list is empty; this would make this error
>         message entirely unnecessary.
>         

Yes, this is a reasonable change.

>         Perhaps this dialog (or the wizard) should mention installing
>         hal if the list is empty (which was the reason the list was
>         empty for me).

Maybe if any hal calls fail, we disable the drop down and put up add a
tooltip or something like that.

> 
> Likewise for #475639... wording changes done, more in-depth changes not.
> 
> For these more in-depth things, if you like the ideas, please let me
> know and I can see about implementing them. If not, then let's close the
> bugs out now. ;)
> 

I'll make a run through the bugs and add specific comments as appropriate.

> In the Shut Down* submenu (and does that really need to be a submenu?)
> and toolbar drop-down, I changed the terminology to "Shut Down" and
> "Force Off". To me, this seems more clear than "Poweroff" vs. "Force
> Poweroff". If you don't like "Force Off", I'd suggest "Power Off"
> instead, keeping the clean "shutdown" action as "Shut Down". I preferred
> "Force Off" over "Power Off" because 1) there is only power to hosts,
> not virtual machines, and 2) I wanted to keep the word "force" to make
> it clear that's not the clean action.
> 

Yes I think the change here is a good idea. Though you missed an
instance manager.py, so that will need to be changed.

> I also updated the machine status terms to be "Shut Down" and "Off"
> instead of "Shutdown" and "Shutoff", respectively, so they match.

Actually the 'Shutdown' in this case was a mistake, that state means the
VM is 'currently shutting down', so 'Shutting Down' would be
appropriate. I'd also prefer to keep "Shutoff' as is: 'off' is pretty
general and at first glance could confuse a user if they didn't realize
it represented VM state, but 'Shutoff' helps get that point across more
clearly.

> 
> * Yes, I also changed "Shutdown" to "Shut Down", which is what GNOME is
> using in the panel.
> 

Yes that looks good.

> Thanks for your time and consideration of these changes,

If you agree with the above changes, respin and resend the patch and
I'll apply it.

Thanks,
Cole