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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
