[Spice-devel] [spice-gtk PATCH 1/3] spicy-connect: Connect dialog changed to window
Pavel Grunt
pgrunt at redhat.com
Mon Jun 8 06:28:09 PDT 2015
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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list