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

Re: [virt-tools-list] [PATCH 4/6] IPv6 support: update the create network wizard



On 03/27/2013 05:37 PM, Gene Czarcinski wrote:
> On 03/26/2013 06:51 PM, Cole Robinson wrote:
>> On 03/26/2013 07:05 AM, Gene Czarcinski wrote:
>>> This update includes changes to both the glade-3 ui and
>>> the releated python code (createnet.py).  There have
>>> been some significant changes made to both files for
>>> IPv6 support and some additional capabilities.
>>>
>>> This update includes the fix so color will work in the
>>> network creation wizard.
>>>
>>> The objective was to get the functionality implemented and
>>> some future effort may be needed to "improve" the
>>> python code.
>>> Signed-off-by: Gene Czarcinski <gene czarc net>
>>> ---
>>>   ui/vmm-create-net.ui     | 1518
>>> ++++++++++++++++++++++++++++++++--------------
>>>   virtManager/createnet.py |  612 +++++++++++++++----
>>>   2 files changed, 1583 insertions(+), 547 deletions(-)
>>>
>> A couple bits here:
>>
>> - The color of the createnet header changed: it used to be black.
> What's wrong with blue? ... OK, it is back to black again.

Blue is the color of the other wizards, but the createnet wizard needs to be
really redesigned to have a more consistent look before switching the color.

>> - There's horizontal separators added everywhere, they are spaced weirdly and
>> don't add anything. Please remove.
> Just following what was there already.  Trying to get things to look good and
> not spread out.  glade-3 is helpful at times and a real PITA at other times.

Hmm, I didn't think there were pre-existing separators, sorry.

>> - Many vbox, like on the 'Name' screen, have a ton of empty entries now. This
>> doesn't affect the visible UI but is clutter when working in glade. Please
>> shrink them down to only the required sizes.
> OK, I got rid of the extras here and elsewhere.
>> - Check the spacing here:
>> http://fedorapeople.org/~crobinso/virt-manager/create-net-spacing.png  . The
>> pre-existing elements don't expand, the added bits do. Also the DNS name field
>> extends across the whole UI. It should probably only be 20 chars long max.
> Twenty characters sounds reasonable but in real life it can be longer.  I am
> not sure how to constrain the gtkentry to just 20 characters or to limit the
> display from consuming the entire line. I will keep "testing" to see if I can
> find a way.

Text entries have a 'width in chars' field which can be in glade. Even if it's
capped at 20, a user can still enter longer text, it will just scroll to the
right. 20 might be too small, so feel free to play with it.

>>
>> Glade can be tricky for some of the subtle things, so make a best effort and
>> if there are still some things you can't get right I'll fix them up before
>> committing.
> I would really like to know/understand how to do some of this with glade-3 and
> the gui.  Even with google being your friend, available documentation is
> limited or does not address what I am trying to do.  The last thing I want to
> do is to dive into the implementing glade/gtk/gdk code to see what can be done
> and how to do it.

Yes this stuff is tricky. Here's the jist

- For that particular case, you likely need to play with the horizontal and
vertical fill/expand settings for every element in the table. For example on
that forwarding page, try and make new elements use the same set of
expand/fill values. Generally you don't want anything to vertically expand.
Also check xalign and yalign

- For getting spacing right and for everything to fit nicely, really just
focus on spacing of containers like boxes, tables, etc. The main content of a
page should be a vbox, every chunk of UI will probably be its own box _within_
that top level vbox. If you add any boxes or tables, make sure their 'spacing'
value is consistent with the rest of the layout, usually 3 pixels for table
column/row spacing, 6 for hbox/vbox, and 12 for the global vbox.

- If you want to indent an entire chunk of UI like a table, don't tweak
spacing/alignment on individual elements, stick the whole thing in an
'alignment' widget, and use that to indent it all with one setting.

- Sometimes there doesn't seem to be any way to get an element to NOT expand,
like a small combo box in a table with a longer widget. You can shrink it, by
sticking the widget in a 2 item box, set it to not expand, stick an
'alignment' in the other entry, and have _that_ expand.

But as I said, don't drive yourself crazy with it, just make a best attempt
and I can try and fix up some of the weird bits as an add on patch. It can be
quite frustrating getting this stuff right.

Thanks,
Cole

>> Please squash this patch together with patch #1, the UI changes aren't much
>> useful in isolation.
>>
>> Also, for testing UI it's quite useful to use the libvirt test driver if you
>> haven't:
>>
>> ./virt-manager --connect test:///`pwd`/tests/testdriver.xml
>>
>> The UI has a couple quirks:
>> - There's a separator between the IPv4 config and IPv6 config, I'd remove it.
>> - Please hide the entire IPv6 frame if the network doesn't have any IPv6 bits.
>> This is similar to how the interface details panel works. The testdriver
>> should help show all the interface details bits
>> - If you have a suite of networks you are using to test the UI, please also
>> add them to tests/testdriver.xml so I can use them as well. It's as simple as
>> just copy+pasting the entire <network> XML into that file, in the same section
>> as the rest of the <network> instances.
>>
>> Thanks,
>> Cole
>>
> I have all the changes done but I want to try an polish this a bit so it will
> be at least a day before I resubmit.
> 
> BTW, I figured out host to run the tests ... it was obvious once I understood
> what was happening.
> 
> Gene
> 


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