[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Cole Robinson wrote:
> 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.
> 

I didn't want these fixes to miss the release, so I tweaked the patch
with my above suggestions and committed.

http://hg.et.redhat.com/cgi-bin/hg-virt.cgi/applications/virt-manager--devel/rev/e2056d9074e5

Thanks,
Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]