[virt-tools-list] [remote-viewer PATCH 1/3] remote-viewer: Connect dialog changed to window
Jonathon Jongsma
jjongsma at redhat.com
Wed Jun 10 19:42:55 UTC 2015
I feel that the way you've split these patches up is a little bit
unusual. The first commit adds the new function (with the same name as
the old function), but does not build the new files. The second commit
adds the files to the build (fortunately, there are no symbol conflicts
because the old connect_dialog() function has static linkage), and then
the third commit removes the old function.
I like the fact that you were trying to split the changes into distinct
commits, but I feel that the way they're currently split actually makes
things a little bit harder to review and bisect. I'd almost prefer a
single commit in this case. I'm also not sure that this function really
needs to be in a separate source file. But I don't feel strongly about
that.
Jonathon
On Wed, 2015-06-10 at 14:47 +0200, Lukas Venhoda wrote:
> Sometimes the connect dialog didn't get a parent.
> It didn't make much sense for it to be a dialog.
>
> Changed the connect dialog to a window.
> Moved the dialog code to its own module.
> Moved the UI definiton into an XML.
>
> Changed response ID from ambiguous -1 and 0 to GTK_RESPONSE_ACCEPT
> and GTK_RESPONSE_REJECT.
>
> Address is now required to connect with enter, or connect button.
> Also sets connect button to non-sensitive, if no text is in adress entry.
>
> Focusing in entries will now unselect recent connection.
> This fixes an issue, when selecting a recent connection, modifying
> the adress entry, and then doubleclicking the selected connection,
> remote-viewer would try to connect to the address in address entry
> instead of the recent connection.
> Doubleclicking teh recent connection will also set address before
> activating, just to be sure.
> ---
> src/remote-viewer-connect.c | 277 ++++++++++++++++++++++++++++++++++++++++++
> src/remote-viewer-connect.h | 36 ++++++
> src/remote-viewer-connect.xml | 152 +++++++++++++++++++++++
> 3 files changed, 465 insertions(+)
> create mode 100644 src/remote-viewer-connect.c
> create mode 100644 src/remote-viewer-connect.h
> create mode 100644 src/remote-viewer-connect.xml
>
> diff --git a/src/remote-viewer-connect.c b/src/remote-viewer-connect.c
> new file mode 100644
> index 0000000..6a0091e
> --- /dev/null
> +++ b/src/remote-viewer-connect.c
> @@ -0,0 +1,277 @@
> +/*
> + * Virt Viewer: A virtual machine console viewer
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include "remote-viewer-connect.h"
> +#include "virt-viewer-util.h"
> +#include <glib/gi18n.h>
> +#include <gdk/gdkkeysyms.h>
> +
> +typedef struct
> +{
> + GtkResponseType response_id;
> + GMainLoop *loop;
> + GtkEntry *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 G_GNUC_UNUSED,
> + GdkEventAny *event G_GNUC_UNUSED,
> + gpointer data)
> +{
> + ConnectionInfo *ci = data;
> + ci->response_id = GTK_RESPONSE_REJECT;
> + shutdown_loop(ci->loop);
> + return TRUE;
> +}
> +
> +static void
> +cancel_button_clicked_cb(GtkButton *button G_GNUC_UNUSED,
> + gpointer data)
> +{
> + ConnectionInfo *ci = data;
> + ci->response_id = GTK_RESPONSE_REJECT;
> + shutdown_loop(ci->loop);
> +}
> +
> +static void
> +connect_button_clicked_cb(GtkButton *button G_GNUC_UNUSED,
> + gpointer data)
> +{
> + ConnectionInfo *ci = data;
> + if (gtk_entry_get_text_length(GTK_ENTRY(ci->entry)) > 0)
> + {
> + ci->response_id = GTK_RESPONSE_ACCEPT;
> + shutdown_loop(ci->loop);
> + }
> +}
> +
> +static void
> +entry_icon_release_cb(GtkEntry* entry,
> + gpointer data G_GNUC_UNUSED)
> +{
> + gtk_entry_set_text(entry, "");
> + gtk_widget_grab_focus(GTK_WIDGET(entry));
> +}
> +
> +static void
> +entry_changed_cb(GtkEditable* entry,
> + gpointer data)
> +{
> + GtkButton *connect_button = data;
> + gboolean rtl = (gtk_widget_get_direction(GTK_WIDGET(entry)) == GTK_TEXT_DIR_RTL);
> + gboolean active = (gtk_entry_get_text_length(GTK_ENTRY(entry)) > 0);
> +
> + gtk_widget_set_sensitive(GTK_WIDGET(connect_button), active);
> +
> + g_object_set(entry,
> + "secondary-icon-name", active ? (rtl ? "edit-clear-rtl-symbolic" : "edit-clear-symbolic") : NULL,
> + "secondary-icon-activatable", active,
> + "secondary-icon-sensitive", active,
> + NULL);
> +}
> +
> +static gboolean
> +entry_focus_in_cb(GtkWidget *widget G_GNUC_UNUSED,
> + GdkEvent *event G_GNUC_UNUSED,
> + gpointer data)
> +{
> + GtkRecentChooser *recent = data;
> + gtk_recent_chooser_unselect_all(recent);
> + return TRUE;
> +}
> +
> +static void
> +entry_activated_cb(GtkEntry *entry G_GNUC_UNUSED,
> + gpointer data)
> +{
> + ConnectionInfo *ci = data;
> + if (gtk_entry_get_text_length(GTK_ENTRY(ci->entry)) > 0)
> + {
> + ci->response_id = GTK_RESPONSE_ACCEPT;
> + shutdown_loop(ci->loop);
> + }
> +}
> +
> +static void
> +recent_item_activated_dialog_cb(GtkRecentChooser *chooser G_GNUC_UNUSED,
> + gpointer data)
> +{
> + ConnectionInfo *ci = data;
> + GtkRecentInfo *info;
> + const gchar *uri;
> +
> + 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);
> +
> + gtk_entry_set_text(ci->entry, uri);
> +
> + gtk_recent_info_unref(info);
> +
> + ci->response_id = GTK_RESPONSE_ACCEPT;
> + shutdown_loop(ci->loop);
> +}
> +
> +static void
> +recent_selection_changed_dialog_cb(GtkRecentChooser *chooser,
> + gpointer data)
> +{
> + GtkRecentInfo *info;
> + const gchar *uri;
> + ConnectionInfo *ci = data;
> +
> + 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);
> +
> + gtk_entry_set_text(ci->entry, uri);
> +
> + gtk_recent_info_unref(info);
> +}
> +
> +static gboolean
> +key_pressed_cb(GtkWidget *widget G_GNUC_UNUSED,
> + GdkEvent *event,
> + gpointer data)
> +{
> + GtkWidget *window = data;
> + gboolean retval;
> + if (event->type == GDK_KEY_PRESS) {
> + switch (event->key.keyval) {
> + case GDK_KEY_Escape:
> + g_signal_emit_by_name(window, "delete-event", NULL, &retval);
> + return TRUE;
> + default:
> + return FALSE;
> + }
> + }
> +
> + return FALSE;
> +}
> +
> +static void
> +make_label_smaller(GtkLabel* label)
> +{
> + PangoAttrList *attributes;
> +
> + attributes = pango_attr_list_new();
> + pango_attr_list_insert(attributes, pango_attr_scale_new(0.9));
> + gtk_label_set_attributes(label, attributes);
> + pango_attr_list_unref(attributes);
> +}
> +
> +gint
> +connect_dialog(gchar **uri G_GNUC_UNUSED)
> +{
> + GtkWidget *window, *connect_button, *cancel_button, *recent, *example_label;
> + GtkRecentFilter *rfilter;
> + GtkBuilder *builder;
> + gboolean active;
> +
> + ConnectionInfo ci = {
> + GTK_RESPONSE_NONE,
> + NULL,
> + NULL
> + };
> +
> + builder = virt_viewer_util_load_ui("remote-viewer-connect.xml");
> + g_return_val_if_fail(builder != NULL, NULL);
> +
> + window = GTK_WIDGET(gtk_builder_get_object(builder, "remote-viewer-connection-window"));
> + connect_button = GTK_WIDGET(gtk_builder_get_object(builder, "connect-button"));
> + cancel_button = GTK_WIDGET(gtk_builder_get_object(builder, "cancel-button"));
> + example_label = GTK_WIDGET(gtk_builder_get_object(builder, "example-label"));
> + ci.entry = GTK_ENTRY(gtk_builder_get_object(builder, "connection-address-entry"));
> +
> + make_label_smaller(GTK_LABEL(example_label));
> +
> + active = (gtk_entry_get_text_length(GTK_ENTRY(ci.entry)) > 0);
> + gtk_widget_set_sensitive(GTK_WIDGET(connect_button), active);
> +
> + recent = GTK_WIDGET(gtk_builder_get_object(builder, "recent-chooser"));
> +
> + rfilter = gtk_recent_filter_new();
> + gtk_recent_filter_add_mime_type(rfilter, "application/x-spice");
> + gtk_recent_filter_add_mime_type(rfilter, "application/x-vnc");
> + gtk_recent_filter_add_mime_type(rfilter, "application/x-virt-viewer");
> + gtk_recent_chooser_set_filter(GTK_RECENT_CHOOSER(recent), rfilter);
> + gtk_recent_chooser_set_local_only(GTK_RECENT_CHOOSER(recent), FALSE);
> +
> + g_signal_connect(window, "delete-event",
> + G_CALLBACK(window_deleted_cb), &ci);
> + g_signal_connect(window, "key-press-event",
> + G_CALLBACK(key_pressed_cb), window);
> + 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.entry, "activate",
> + G_CALLBACK(entry_activated_cb), &ci);
> + g_signal_connect(ci.entry, "changed",
> + G_CALLBACK(entry_changed_cb), connect_button);
> + g_signal_connect(ci.entry, "icon-release",
> + G_CALLBACK(entry_icon_release_cb), ci.entry);
> + g_signal_connect(ci.entry, "focus-in-event",
> + G_CALLBACK(entry_focus_in_cb), recent);
> +
> + 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);
> +
> + gtk_widget_show_all(window);
> +
> + ci.loop = g_main_loop_new(NULL, FALSE);
> + g_main_loop_run(ci.loop);
> +
> + if (ci.response_id == GTK_RESPONSE_ACCEPT) {
> + *uri = g_strdup(gtk_entry_get_text(GTK_ENTRY(ci.entry)));
> + g_strstrip(*uri);
> + } else {
> + *uri = NULL;
> + }
> +
> + g_object_unref(builder);
> + gtk_widget_destroy(window);
> +
> + return ci.response_id;
> +}
> +
> +/*
> + * Local variables:
> + * c-indent-level: 4
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/src/remote-viewer-connect.h b/src/remote-viewer-connect.h
> new file mode 100644
> index 0000000..cc4776b
> --- /dev/null
> +++ b/src/remote-viewer-connect.h
> @@ -0,0 +1,36 @@
> +/*
> + * Virt Viewer: A virtual machine console viewer
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#ifndef REMOTE_VIEWER_CONNECT_H
> +#define REMOTE_VIEWER_CONNECT_H
> +
> +#include <gtk/gtk.h>
> +
> +gint connect_dialog(gchar **uri);
> +
> +#endif /* REMOTE_VIEWER_CONNECT_H */
> +
> +/*
> + * Local variables:
> + * c-indent-level: 4
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/src/remote-viewer-connect.xml b/src/remote-viewer-connect.xml
> new file mode 100644
> index 0000000..dcd14cf
> --- /dev/null
> +++ b/src/remote-viewer-connect.xml
> @@ -0,0 +1,152 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<!-- Generated with glade 3.18.3 -->
> +<interface>
> + <object class="GtkWindow" id="remote-viewer-connection-window">
> + <property name="can_focus">False</property>
> + <property name="title" translatable="yes">Connection details</property>
> + <child>
> + <object class="GtkVBox" id="main-box">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="border_width">10</property>
> + <property name="spacing">20</property>
> + <child>
> + <object class="GtkVBox" id="connection-address-box">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="spacing">6</property>
> + <child>
> + <object class="GtkLabel" id="connection-address-label">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="label" translatable="yes">Connection Address</property>
> + <property name="xalign">0</property>
> + <attributes>
> + <attribute name="weight" value="bold"/>
> + </attributes>
> + </object>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">True</property>
> + <property name="position">0</property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkEntry" id="connection-address-entry">
> + <property name="visible">True</property>
> + <property name="can_focus">True</property>
> + </object>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">True</property>
> + <property name="position">1</property>
> + </packing>
> + </child>
> + <child>
> + <object class="GtkLabel" id="example-label">
> + <property name="visible">True</property>
> + <property name="can_focus">False</property>
> + <property name="xalign">0</property>
> + <property name="sensitive">False</property>
> + <property name="label" translatable="yes">For example, spice://foo.example.org:5900</property>
> + </object>
> + <packing>
> + <property name="expand">False</property>
> + <property name="fill">True</property>
> + <property name="position">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>
> + <property name="spacing">6</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>
> + <property name="xalign">0</property>
> + <attributes>
> + <attribute name="weight" value="bold"/>
> + </attributes>
> + </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="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">2</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">6</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">3</property>
> + </packing>
> + </child>
> + </object>
> + </child>
> + </object>
> +</interface>
> --
> 2.4.2
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
More information about the virt-tools-list
mailing list