[virt-tools-list] [PATCH v2] Avoid creating ovirt foreign menu item manually

[Christophe Fergeau]

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) <etrunko at 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 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: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20160623/b6f9f61f/attachment.sig>