This looks fine to me, however I'd rephrase the shortlog (or add to the
long log) that we get the Change CD menu from the UI file rather than
manually creating it. Before looking at the commit, I was not sure what
this was about.

Christophe

On Thu, Jun 23, 2016 at 11:23:11AM -0300, Eduardo Lima (Etrunko) wrote:
> This patch is a preparation for a upcoming UI change that will present
> the ISO list in a separate dialog, instead of a submenu.
> 
> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com>
> ---
>  src/remote-viewer.c             | 23 +++++++++++------------
>  src/resources/ui/virt-viewer.ui |  8 ++++++++
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> index 71723cf..6d29bf2 100644
> --- a/src/remote-viewer.c
> +++ b/src/remote-viewer.c
> @@ -738,29 +738,28 @@ ovirt_foreign_menu_update(GtkApplication *gtkapp, 
> GtkWindow *gtkwin, G_GNUC_UNUS
>      VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
> "virt-viewer-window");
>      GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
>      GtkWidget *submenu;
> -    GtkMenuShell *shell = 
> GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
> "top-menu"));
>  
>      if (app->priv->ovirt_foreign_menu == NULL) {
>          /* nothing to do */
>          return;
>      }
> -    if (menu == NULL) {
> -        menu = gtk_menu_item_new_with_label(_("_Change CD"));
> -        gtk_menu_item_set_use_underline(GTK_MENU_ITEM(menu), TRUE);
> -        gtk_menu_shell_append(shell, menu);
> -        g_object_set_data_full(G_OBJECT(win), "foreign-menu",
> -                               g_object_ref(menu),
> -                               (GDestroyNotify)gtk_widget_destroy);
> -    }
>  
>      submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
> -    if (submenu != NULL) {
> -        gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
> -    } else {
> +    if (submenu == NULL) {
>          /* No items to show, no point in showing the menu */
> +        if (menu != NULL)
> +           gtk_widget_set_visible(menu, FALSE);
>          g_object_set_data(G_OBJECT(win), "foreign-menu", NULL);
> +        return;
> +    }
> +
> +    if (menu == NULL) {
> +        menu = 
> GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
> "menu-change-cd"));
> +        g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
> +        gtk_widget_set_visible(menu, TRUE);
>      }
>  
> +    gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
>      gtk_widget_show_all(menu);
>  }
>  
> diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
> index 830a451..219e0af 100644
> --- a/src/resources/ui/virt-viewer.ui
> +++ b/src/resources/ui/virt-viewer.ui
> @@ -243,6 +243,14 @@
>                  </child>
>                </object>
>              </child>
> +            <child>
> +              <object class="GtkMenuItem" id="menu-change-cd">
> +                <property name="use_action_appearance">False</property>
> +                <property name="can_focus">False</property>
> +                <property name="label" translatable="yes">_Change 
> CD</property>
> +                <property name="use_underline">True</property>
> +              </object>
> +            </child>
>            </object>
>            <packing>
>              <property name="expand">False</property>
> -- 
> 2.5.5
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list

Attachment: signature.asc
Description: PGP signature

_______________________________________________
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Reply via email to