[virt-tools-list] [virt-manager PATCH v2] Redesign OS distro selection UI to be faster to use

[Daniel P. Berrangé]

On Sun, Apr 29, 2018 at 12:47:56AM +0100, Radostin Stoyanov wrote:
> 1. I noticed that when uncheck "install-detect-os" (Automatically detect
> from the installation media / source) and then press "Back" followed by
> "Forward" the inactive state of the "install-detect-os" checkbox will be
> preserved, but the input field for "install-os-name" SearchEntry will
> remain disabled/not sensitive (even after providing media/source). This
> can be solved with the following diff:
> 
> diff --git a/virtManager/create.py b/virtManager/create.py
> index ac252dda..3a1c3666 100644
> --- a/virtManager/create.py
> +++ b/virtManager/create.py
> @@ -2029,8 +2029,11 @@ class vmmCreate(vmmGObjectUI):
>          if check_install_page and not is_install_page:
>              return
>          if not media:
> +            autodetect = self.widget("install-detect-os").get_active()
>              name = self.widget("install-os-name")
> -            if not name.get_sensitive():
> +            if not autodetect:
> +                name.set_sensitive(True)
> +            else:
>                  name.set_text(_("Waiting for install media / source"))
>              return
>          if not self._is_os_detect_active():

This breaks the PXE page - it needs to be fixed in _set_install_page
instead.

> 2. In my opinion, when reading the content of "install-os-name"
> SearchEntry we should apply .strip() to remove trailing/leading
> white-space. Otherwise no results will be shown.

Yes.

> 3. I think it would be better to set the vertical scroll policy of
> GtkScrolledWindow (contains the OS instances) to Automatic. In this way,
> vertical scroll bar will be show only when needed.

Automatic scrollbars are ok when the list you're displaying is
filled once. For this list though, we're dynamically changing
the contents on every key press, which will cause the scroll
bars to continuall show & hide themselves which I find pretty
unpleasant experience.

> 4. Simplify _os_name_search_changed() ?

I've done something similar which avoids repeating the
same conditional check many times.

> 5. What do you think about adding horizontal grid lines?

Sure.


> 6. Instead of connecting to the "changed" signal of
> Gtk|||TreeSelection|, we can use "row_activated" for GtkTreeView. This
> will enable the hover functionality for rows, IMHO looks better.

Yeah, that looks ok.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|