Re: [spice-gtk PATCH 1/3] spicy-connect: Connect dialog changed to window

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]