[virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt foreign menu with dedicated dialog

[Eduardo Lima (Etrunko)]

On 08/02/2016 01:17 PM, Christophe Fergeau wrote:
> On Fri, Jul 29, 2016 at 06:40:23PM -0300, Eduardo Lima (Etrunko) wrote:
>> I have pushed the first two patches of the series because they were
>> already acknowledged and were pretty much self-contained.
>>
>> I tried to replace GtkTreeView in favor of new GtkListBox, but after some
>> painful work, I decided to drop it because it is not possible to
>> reproduce the exact same behavior of the former with the latter. For
>> instance, once we have a GtkRadioButton activated, it is not possible to
>> deactivate it. 
> 
> For what it's worth, I'm not sure we should keep GtkRadioButtons at all
> cost in the list, imo they look not so nice. Dunno if it would be
> possibe to have either empty column or check mark instead of a
> GtkRadioButton.
> 

Well, I am no UI expert, but it seems to me that radio buttons are the
best choice to express the behavior of the list, exactly 1 or 0 item can
be active at any given time. With check buttons, user is given the
impression that more than once can be selected, which is not the case.

All in all it is a one line change to show check buttons instead of
radio buttons.

>>
>> In this version:
>>
>>  * Removed leftover enum when changed from GtkNotebook to GtkStack
>>  * Some cosmetic fixes (indentation, renaming, etc)
>>  * UI Tweaks:
>>    - Added some padding between items of the list.
>>    - Set tree view selection to current ISO when the list is first loaded
>>      or refreshed.
>>    - Handle tree view "activate" signal the same way as radio button
>>      toggle
>>    - Removed "Select ISO" label and alignment in header bar patch.
>>
>> Eduardo Lima (Etrunko) (10):
>>   ovirt-foreign-menu: Remove timer used to refresh iso list
>>   ovirt-foreign-menu: Add accessors for current iso and iso list
>>   ovirt-foreign-menu: Remove GtkMenu related functions
>>   ovirt-foreign-menu: Notify of new files even if nothing changed
>>   UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
>>   Introduce ISO List dialog
>>   Run iso-dialog when 'Change CD' menu is activated
>>   remote-viewer: Make ovirt-foreign-menu a property
>>   iso-dialog: Implement functionality provided by oVirt foreign menu
>>   iso-dialog: Use header bar for buttons
> 
> I'm not sure how to approach this series as I don't think
> 'ovirt-foreign-menu: Add accessors for current iso and iso list'
> 'ovirt-foreign-menu: Notify of new files even if nothing changed'
> are needed if you want to have proper error handling in the dialog.
> So I don't think I'd have much more to add compared to the first
> iteration of the review. Or are there specific changes you want me to
> look at?

I have been busy with other things at the moment, and I could not finish
the work here. The idea was to gather comments about UI and behavior in
general, as the new changes will not affect the appearance of the
dialog. So it seems it is looking good, apart from the Radio/Check
buttons decision.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20160803/177dad33/attachment.sig>