[virt-tools-list] [PATCH virt-viewer 6/9] iso-dialog: Implement functionality provided by oVirt foreign menu

Christophe Fergeau cfergeau at redhat.com
Thu Jan 19 12:48:42 UTC 2017


The issues pointed out in
https://www.redhat.com/archives/virt-tools-list/2017-January/msg00039.html
do not seem to have been addressed?

Did you try merging the patches from "Introduce ISO List dialog" to "Run
iso-dialog when 'Change CD' menu is activated" (without the headerbar
change)? I think they would make more sense together, did not check if
that would make the diff far too big or not.

Christophe

On Wed, Jan 18, 2017 at 12:16:57PM -0200, Eduardo Lima (Etrunko) wrote:
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
> ---
>  src/remote-viewer-iso-list-dialog.c        | 204 ++++++++++++++++++++++++++++-
>  src/resources/ui/remote-viewer-iso-list.ui |   5 +-
>  2 files changed, 205 insertions(+), 4 deletions(-)
> 
> diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c
> index 858719c..ef60854 100644
> --- a/src/remote-viewer-iso-list-dialog.c
> +++ b/src/remote-viewer-iso-list-dialog.c
> @@ -26,6 +26,9 @@
>  #include "virt-viewer-util.h"
>  #include "ovirt-foreign-menu.h"
>  
> +static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self);
> +static void remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self, gchar *message);
> +
>  G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG)
>  
>  #define DIALOG_PRIVATE(o) \
> @@ -33,17 +36,32 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE
>  
>  struct _RemoteViewerISOListDialogPrivate
>  {
> +    GtkListStore *list_store;
>      GtkWidget *stack;
> +    GtkWidget *tree_view;
>      OvirtForeignMenu *foreign_menu;
>  };
>  
> +enum RemoteViewerISOListDialogModel
> +{
> +    ISO_IS_ACTIVE = 0,
> +    ISO_NAME,
> +    FONT_WEIGHT,
> +};
> +
> +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer, gchar *path, gpointer user_data);
> +void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view, GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data);
> +
>  static void
>  remote_viewer_iso_list_dialog_dispose(GObject *object)
>  {
>      RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
>      RemoteViewerISOListDialogPrivate *priv = self->priv;
>  
> -    g_clear_object(&priv->foreign_menu);
> +    if (priv->foreign_menu) {
> +        g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
> +        g_clear_object(&priv->foreign_menu);
> +    }
>      G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
>  }
>  
> @@ -58,6 +76,74 @@ remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
>  }
>  
>  static void
> +remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
> +{
> +    RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
> +    gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list",
> +                                     GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);

I don't think I'd use animations here, I'd just switch from spinner to
iso list, I find the animation distracting.

> +    gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE);
> +}
> +
> +static void
> +remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self)
> +{
> +    RemoteViewerISOListDialogPrivate *priv = self->priv;
> +    gchar *current_iso = ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu);
> +    gboolean active = g_strcmp0(current_iso, name) == 0;
> +    gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
> +    GtkTreeIter iter;
> +
> +    gtk_list_store_append(priv->list_store, &iter);
> +    gtk_list_store_set(priv->list_store, &iter,
> +                       ISO_IS_ACTIVE, active,
> +                       ISO_NAME, name,
> +                       FONT_WEIGHT, weight, -1);
> +
> +    if (active) {
> +        GtkTreePath *path = gtk_tree_model_get_path(GTK_TREE_MODEL(priv->list_store), &iter);
> +        gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, FALSE);
> +        gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, NULL, TRUE, 0.5, 0.5);
> +        gtk_tree_path_free(path);
> +    }
> +
> +    g_free(current_iso);
> +}
> +
> +static void
> +fetch_iso_names_cb(OvirtForeignMenu *foreign_menu,
> +                   GAsyncResult *result,
> +                   RemoteViewerISOListDialog *self)
> +{
> +    RemoteViewerISOListDialogPrivate *priv = self->priv;
> +    GError *error = NULL;
> +    GList *iso_list;
> +
> +    iso_list = ovirt_foreign_menu_fetch_iso_names_finish(foreign_menu, result, &error);
> +
> +    if (!iso_list) {
> +        remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to fetch CD names"));
> +        g_clear_error(&error);
> +        return;
> +    }
> +
> +    iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu);
> +    g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self);
> +    remote_viewer_iso_list_dialog_show_files(self);
> +}
> +
> +
> +static void
> +remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self)
> +{
> +    RemoteViewerISOListDialogPrivate *priv = self->priv;
> +
> +    gtk_list_store_clear(priv->list_store);
> +    ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL,
> +                                             (GAsyncReadyCallback) fetch_iso_names_cb,
> +                                             self);
> +}
> +
> +static void
>  remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
>                                         gint response_id,
>                                         gpointer user_data G_GNUC_UNUSED)
> @@ -71,7 +157,47 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
>      gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
>                                       GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
>      gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> -    gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE);
> +    remote_viewer_iso_list_dialog_refresh_iso_list(self);
> +}
> +
> +void
> +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED,
> +                                      gchar *path,
> +                                      gpointer user_data)
> +{
> +    RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data);
> +    RemoteViewerISOListDialogPrivate *priv = self->priv;
> +    GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
> +    GtkTreePath *tree_path = gtk_tree_path_new_from_string(path);
> +    GtkTreeIter iter;
> +    gboolean active;
> +    gchar *name;
> +
> +    gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), tree_path, NULL, FALSE);
> +    gtk_tree_model_get_iter(model, &iter, tree_path);
> +    gtk_tree_model_get(model, &iter,
> +                       ISO_IS_ACTIVE, &active,
> +                       ISO_NAME, &name, -1);
> +
> +    gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> +    gtk_widget_set_sensitive(priv->tree_view, FALSE);
> +
> +    ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL,
> +                                                  (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed,
> +                                                  self);
> +    gtk_tree_path_free(tree_path);
> +    g_free(name);


That is quite similar to remote_viewer_iso_list_dialog_foreach(),
dunno if part of the code could be shared?

> +}
> +
> +void
> +remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view G_GNUC_UNUSED,
> +                                            GtkTreePath *path,
> +                                            GtkTreeViewColumn *col G_GNUC_UNUSED,
> +                                            gpointer user_data)
> +{
> +    gchar *path_str = gtk_tree_path_to_string(path);
> +    remote_viewer_iso_list_dialog_toggled(NULL, path_str, user_data);
> +    g_free(path_str);
>  }
>  
>  static void
> @@ -80,12 +206,19 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
>      GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
>      RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
>      GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
> +    GtkCellRendererToggle *cell_renderer;
>  
>      gtk_builder_connect_signals(builder, self);
>  
>      priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack"));
>      gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0);
>  
> +    priv->list_store = GTK_LIST_STORE(gtk_builder_get_object(builder, "liststore"));
> +    priv->tree_view = GTK_WIDGET(gtk_builder_get_object(builder, "view"));
> +    cell_renderer = GTK_CELL_RENDERER_TOGGLE(gtk_builder_get_object(builder, "cellrenderertoggle"));
> +    gtk_cell_renderer_toggle_set_radio(cell_renderer, TRUE);
> +    gtk_cell_renderer_set_padding(GTK_CELL_RENDERER(cell_renderer), 6, 6);
> +
>      g_object_unref(builder);
>  
>      gtk_dialog_add_buttons(GTK_DIALOG(self),
> @@ -95,10 +228,74 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
>  
>      gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE);
>      gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE);
> -    gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE);
>      g_signal_connect(self, "response", G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL);
>  }
>  
> +static void
> +remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self,
> +                                         gchar *message)
> +{
> +    GtkWidget *dialog;
> +
> +    g_warn_if_fail(message != NULL);
> +
> +    dialog = gtk_message_dialog_new(GTK_WINDOW(self),
> +                                    GTK_DIALOG_DESTROY_WITH_PARENT,
> +                                    GTK_MESSAGE_ERROR,
> +                                    GTK_BUTTONS_CLOSE,
> +                                    message ? message : _("Unspecified error"));
> +    gtk_dialog_run(GTK_DIALOG(dialog));
> +    gtk_widget_destroy(dialog);
> +}
> +
> +static void
> +ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu,
> +                                    GAsyncResult *result,
> +                                    RemoteViewerISOListDialog *self)
> +{
> +    RemoteViewerISOListDialogPrivate *priv = self->priv;
> +    GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
> +    gchar *current_iso;
> +    GtkTreeIter iter;
> +    gchar *name;
> +    gboolean active, match = FALSE;
> +    GError *error = NULL;
> +
> +    if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, &error)) {
> +        remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD"));
> +        g_clear_error(&error);
> +    }

I think it's intentional that we don't return early on failure so that
we reselect the correct ISO, maybe it would be worth a comment?

> +
> +    if (!gtk_tree_model_get_iter_first(model, &iter))
> +        return;
> +
> +    current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
> +
> +    do {
> +        gtk_tree_model_get(model, &iter,
> +                           ISO_IS_ACTIVE, &active,
> +                           ISO_NAME, &name, -1);
> +        match = g_strcmp0(current_iso, name) == 0;
> +
> +        /* iso is not active anymore */
> +        if (active && !match) {
> +            gtk_list_store_set(priv->list_store, &iter,
> +                               ISO_IS_ACTIVE, FALSE,
> +                               FONT_WEIGHT, PANGO_WEIGHT_NORMAL, -1);
> +        } else if (match) {
> +            gtk_list_store_set(priv->list_store, &iter,
> +                               ISO_IS_ACTIVE, TRUE,
> +                               FONT_WEIGHT, PANGO_WEIGHT_BOLD, -1);
> +        }
> +
> +        g_free(name);
> +    } while (gtk_tree_model_iter_next(model, &iter));
> +
> +    gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE);
> +    gtk_widget_set_sensitive(priv->tree_view, TRUE);
> +    g_free(current_iso);
> +}
> +
>  GtkWidget *
>  remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
>  {
> @@ -117,5 +314,6 @@ remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
>  
>      self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
>      self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
> +    remote_viewer_iso_list_dialog_refresh_iso_list(self);
>      return dialog;
>  }
> diff --git a/src/resources/ui/remote-viewer-iso-list.ui b/src/resources/ui/remote-viewer-iso-list.ui
> index 624791f..ab1bdc4 100644
> --- a/src/resources/ui/remote-viewer-iso-list.ui
> +++ b/src/resources/ui/remote-viewer-iso-list.ui
> @@ -106,6 +106,7 @@
>                      <property name="rules_hint">True</property>
>                      <property name="search_column">1</property>
>                      <property name="enable_grid_lines">horizontal</property>
> +                    <signal name="row-activated" handler="remote_viewer_iso_list_dialog_row_activated" swapped="no"/>
>                      <child internal-child="selection">
>                        <object class="GtkTreeSelection" id="treeview-selection"/>
>                      </child>
> @@ -114,7 +115,9 @@
>                          <property name="sizing">autosize</property>
>                          <property name="title" translatable="yes">Selected</property>
>                          <child>
> -                          <object class="GtkCellRendererToggle" id="cellrenderertoggle"/>
> +                          <object class="GtkCellRendererToggle" id="cellrenderertoggle">
> +                            <signal name="toggled" handler="remote_viewer_iso_list_dialog_toggled" swapped="no"/>
> +                          </object>
>                            <attributes>
>                              <attribute name="active">0</attribute>
>                            </attributes>
> -- 
> 2.9.3
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170119/30d7bd44/attachment.sig>


More information about the virt-tools-list mailing list