Hi Lukas, I know it is acked and that spicy is just a test tool, but I have some comments... The dialog looks different in gkt2 than in gtk3 (also different from the hardcoded dialog - the text is centered, smaller margins). Few comments inline Pavel On Mon, 2015-06-08 at 11:36 +0200, Lukas Venhoda wrote: > Connect dialog in spicy had no transinient parent. > It didn't make much sense for it to be a dialog. > > Changed the dialog to window. > Moved UI definition from code to XML. > Moved dialog to its own module. > --- > gtk/spicy-connect.c | 197 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > gtk/spicy-connect.h | 26 +++++++ > gtk/spicy-connect.xml | 175 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 398 insertions(+) > create mode 100644 gtk/spicy-connect.c > create mode 100644 gtk/spicy-connect.h > create mode 100644 gtk/spicy-connect.xml > > diff --git a/gtk/spicy-connect.c b/gtk/spicy-connect.c > new file mode 100644 > index 0000000..c221739 > --- /dev/null > +++ b/gtk/spicy-connect.c > @@ -0,0 +1,197 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2010-2015 Red Hat, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see <http://www.gnu.org/licenses/ > >. > +*/ > + > +#include <gtk/gtk.h> > +#include "spicy-connect.h" > + > +typedef enum { > + CANCEL_RESPONSE = -1, > + CONNECT_RESPONSE > +}ResponseID; space, what about using Gtk response types (e.g. GTK_RESPONSE_ACCEPT) ? > + > +typedef struct > +{ > + ResponseID response_id; > + GMainLoop *loop; > + SpiceSession *session; > + GtkEntry *host_entry, *port_entry, *tls_entry; > +} ConnectionInfo; > + > +static void > +shutdown_loop(GMainLoop *loop) > +{ > + if (g_main_loop_is_running(loop)) > + g_main_loop_quit(loop); > +} > + > +static gboolean > +window_deleted_cb(GtkWindow *window, GdkEventAny *event, gpointer data) > +{ > + ConnectionInfo *ci = data; > + ci->response_id = CANCEL_RESPONSE; > + shutdown_loop(ci->loop); > + return TRUE; > +} > + > +static void > +cancel_button_clicked_cb(GtkButton *button, gpointer data) > +{ > + ConnectionInfo *ci = data; > + ci->response_id = CANCEL_RESPONSE; > + shutdown_loop(ci->loop); > +} > + > +static void > +set_connection_info(ConnectionInfo *ci) > +{ > + SpiceSession *session = ci->session; > + > + const gchar *txt; > + txt = gtk_entry_get_text(ci->host_entry); > + g_object_set(session, "host", txt, NULL); > + > + txt = gtk_entry_get_text(ci->port_entry); > + g_object_set(session, "port", txt, NULL); > + > + txt = gtk_entry_get_text(ci->tls_entry); > + g_object_set(session, "tls-port", txt, NULL); > +} > + > +static void > +entry_activated_cb(GtkEntry *entry, gpointer data) > +{ > + ConnectionInfo *ci = data; > + ci->response_id = CONNECT_RESPONSE; > + set_connection_info(ci); > + shutdown_loop(ci->loop); > +} > + > +static void > +connect_button_clicked_cb(GtkButton *button, gpointer data) > +{ > + ConnectionInfo *ci = data; > + ci->response_id = CONNECT_RESPONSE; > + set_connection_info(ci); > + shutdown_loop(ci->loop); > +} > + > +#ifndef G_OS_WIN32 > +static void > +recent_item_activated_dialog_cb(GtkRecentChooser *chooser, gpointer data) > +{ > + ConnectionInfo *ci = data; > + ci->response_id = CONNECT_RESPONSE; > + set_connection_info(ci); > + shutdown_loop(ci->loop); > +} > + > +static void > +recent_selection_changed_dialog_cb(GtkRecentChooser *chooser, gpointer data) > +{ > + GtkRecentInfo *info; > + gchar *txt = NULL; > + const gchar *uri; > + ConnectionInfo *ci = data; > + SpiceSession *session = ci->session; > + > + info = gtk_recent_chooser_get_current_item(chooser); > + if (info == NULL) > + return; > + > + uri = gtk_recent_info_get_uri(info); > + g_return_if_fail(uri != NULL); > + > + g_object_set(session, "uri", uri, NULL); > + > + g_object_get(session, "host", &txt, NULL); > + gtk_entry_set_text(ci->host_entry, txt ? txt : ""); > + g_free(txt); > + > + g_object_get(session, "port", &txt, NULL); > + gtk_entry_set_text(ci->port_entry, txt ? txt : ""); > + g_free(txt); > + > + g_object_get(session, "tls-port", &txt, NULL); > + gtk_entry_set_text(ci->tls_entry, txt ? txt : ""); > + g_free(txt); > + > + gtk_recent_info_unref(info); > +} > +#endif > + > +int > +connect_dialog(SpiceSession *session) > +{ > + GtkWidget *window, *connect_button, *cancel_button; > + GtkBuilder *builder; > + > + builder = gtk_builder_new(); > + gtk_builder_add_from_file(builder, "spicy-connect.xml", NULL); You should check for the error and return if failed. > + > + window = GTK_WIDGET(gtk_builder_get_object(builder, "spicy-connection > -window")); > + connect_button = GTK_WIDGET(gtk_builder_get_object(builder, "connect > -button")); > + cancel_button = GTK_WIDGET(gtk_builder_get_object(builder, "cancel > -button")); > + > + ConnectionInfo ci = { It should be declared on the top of the block. > + CANCEL_RESPONSE, > + NULL, > + session, > + GTK_ENTRY(gtk_builder_get_object(builder, "hostname-entry")), > + GTK_ENTRY(gtk_builder_get_object(builder, "port-entry")), > + GTK_ENTRY(gtk_builder_get_object(builder, "tls-port-entry")) > + }; > + > + g_signal_connect(window, "delete-event", > + G_CALLBACK(window_deleted_cb), &ci); > + g_signal_connect(connect_button, "clicked", > + G_CALLBACK(connect_button_clicked_cb), &ci); > + g_signal_connect(cancel_button, "clicked", > + G_CALLBACK(cancel_button_clicked_cb), &ci); > + > + g_signal_connect(ci.host_entry, "activate", > + G_CALLBACK(entry_activated_cb), &ci); > + g_signal_connect(ci.port_entry, "activate", > + G_CALLBACK(entry_activated_cb), &ci); > + g_signal_connect(ci.tls_entry, "activate", > + G_CALLBACK(entry_activated_cb), &ci); > + > +#ifndef G_OS_WIN32 > + GtkWidget *recent; > + GtkRecentFilter *rfilter; > + > + recent = GTK_WIDGET(gtk_builder_get_object(builder, "recent-chooser")); > + gtk_widget_set_visible(recent, TRUE); > + > + rfilter = gtk_recent_filter_new(); > + gtk_recent_filter_add_mime_type(rfilter, "application/x-spice"); > + gtk_recent_chooser_set_filter(GTK_RECENT_CHOOSER(recent), rfilter); > + gtk_recent_chooser_set_local_only(GTK_RECENT_CHOOSER(recent), FALSE); > + g_signal_connect(recent, "selection-changed", > G_CALLBACK(recent_selection_changed_dialog_cb), &ci); > + g_signal_connect(recent, "item-activated", > G_CALLBACK(recent_item_activated_dialog_cb), &ci); > +#endif > + > + gtk_widget_show_all(window); > + > + ci.loop = g_main_loop_new(NULL, FALSE); > + g_main_loop_run(ci.loop); > + > + g_object_unref(builder); > + gtk_widget_destroy(window); > + > + return ci.response_id; > +} > diff --git a/gtk/spicy-connect.h b/gtk/spicy-connect.h > new file mode 100644 > index 0000000..3f1869d > --- /dev/null > +++ b/gtk/spicy-connect.h > @@ -0,0 +1,26 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2010-2015 Red Hat, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see <http://www.gnu.org/licenses/ > >. > +*/ > + > +#ifndef SPICY_CONNECT_H > +#define SPICY_CONNECT_H > + > +#include "spice-widget.h" > + > +int connect_dialog(SpiceSession *session); > + > +#endif > diff --git a/gtk/spicy-connect.xml b/gtk/spicy-connect.xml > new file mode 100644 > index 0000000..1e07f5b > --- /dev/null > +++ b/gtk/spicy-connect.xml > @@ -0,0 +1,175 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<!-- Generated with glade 3.18.3 --> > +<interface> > + <object class="GtkWindow" id="spicy-connection-window"> > + <property name="can_focus">False</property> > + <property name="title" translatable="yes">Connect to SPICE</property> > + <child> > + <object class="GtkVBox" id="main-box"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="spacing">5</property> > + <child> > + <object class="GtkTable" id="table"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="row_spacing">5</property> > + <property name="n_rows">3</property> > + <property name="n_columns">2</property> > + <child> > + <object class="GtkLabel" id="hostname-label"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="xpad">10</property> > + <property name="label" translatable="yes">Hostname</property> > + <property name="justify">right</property> > + </object> > + <packing> > + <property name="left_attach">0</property> > + <property name="top_attach">0</property> > + </packing> > + </child> > + <child> > + <object class="GtkLabel" id="port-label"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="label" translatable="yes">Port</property> > + <property name="justify">right</property> > + </object> > + <packing> > + <property name="left_attach">0</property> > + <property name="top_attach">1</property> > + </packing> > + </child> > + <child> > + <object class="GtkLabel" id="tls-port-label"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="label" translatable="yes">TLS Port</property> > + <property name="justify">right</property> > + </object> > + <packing> > + <property name="left_attach">0</property> > + <property name="top_attach">2</property> > + </packing> > + </child> > + <child> > + <object class="GtkEntry" id="hostname-entry"> > + <property name="visible">True</property> > + <property name="can_focus">True</property> > + </object> > + <packing> > + <property name="left_attach">1</property> > + <property name="top_attach">0</property> > + </packing> > + </child> > + <child> > + <object class="GtkEntry" id="port-entry"> > + <property name="visible">True</property> > + <property name="can_focus">True</property> > + </object> > + <packing> > + <property name="left_attach">1</property> > + <property name="top_attach">1</property> > + </packing> > + </child> > + <child> > + <object class="GtkEntry" id="tls-port-entry"> > + <property name="visible">True</property> > + <property name="can_focus">True</property> > + </object> > + <packing> > + <property name="left_attach">1</property> > + <property name="top_attach">2</property> > + </packing> > + </child> > + </object> > + <packing> > + <property name="expand">False</property> > + <property name="fill">True</property> > + <property name="position">0</property> > + </packing> > + </child> > + <child> > + <object class="GtkVBox" id="recent-chooser-box"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <child> > + <object class="GtkLabel" id="recent-chooser-label"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="label" translatable="yes">Recent > connections:</property> > + </object> > + <packing> > + <property name="expand">False</property> > + <property name="fill">True</property> > + <property name="position">0</property> > + </packing> > + </child> > + <child> > + <object class="GtkRecentChooserWidget" id="recent-chooser"> > + <property name="visible">False</property> > + <property name="can_focus">False</property> > + <property name="limit">20</property> > + <property name="local_only">False</property> > + <property name="show_icons">False</property> > + <property name="sort_type">mru</property> > + </object> > + <packing> > + <property name="expand">True</property> > + <property name="fill">True</property> > + <property name="position">1</property> > + </packing> > + </child> > + </object> > + <packing> > + <property name="expand">True</property> > + <property name="fill">True</property> > + <property name="position">1</property> > + </packing> > + </child> > + <child> > + <object class="GtkHButtonBox" id="button-box"> > + <property name="visible">True</property> > + <property name="can_focus">False</property> > + <property name="resize_mode">immediate</property> > + <property name="spacing">5</property> > + <property name="layout_style">end</property> > + <child> > + <object class="GtkButton" id="cancel-button"> > + <property name="label" translatable="yes">Cancel</property> > + <property name="visible">True</property> > + <property name="can_focus">True</property> > + <property name="receives_default">True</property> > + </object> > + <packing> > + <property name="expand">True</property> > + <property name="fill">True</property> > + <property name="position">0</property> > + </packing> > + </child> > + <child> > + <object class="GtkButton" id="connect-button"> > + <property name="label" translatable="yes">Connect</property> > + <property name="visible">True</property> > + <property name="can_focus">True</property> > + <property name="receives_default">True</property> > + </object> > + <packing> > + <property name="expand">True</property> > + <property name="fill">True</property> > + <property name="position">1</property> > + </packing> > + </child> > + </object> > + <packing> > + <property name="expand">False</property> > + <property name="fill">False</property> > + <property name="pack_type">end</property> > + <property name="position">2</property> > + </packing> > + </child> > + </object> > + </child> > + </object> > +</interface> > -- > 2.4.2 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel