GNOME.org
 

Re: [Evolution-hackers] Status bar

On Thu, 2003-12-04 at 22:59, ERDI Gergo wrote: 
On Wed, 3 Dec 2003, Ettore Perazzoli wrote:

> > I'll do this and then modify em-folder-view to use the BonoboUI <status>
> > mechanism when in a standalone message window, but e_activity_handler_*
> > when embedded in the main Evolution shell. How does that sound?
> 
> Sounds good to me.

so how about this one:

Mostly style and design comments, the logic looks ok.  although i don't grok the need for two ways to set the status.

Index: mail/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.2930
diff -u -r1.2930 ChangeLog
--- mail/ChangeLog	3 Dec 2003 19:24:34 -0000	1.2930
+++ mail/ChangeLog	4 Dec 2003 11:57:17 -0000
@@ -1,3 +1,19 @@
+2003-12-04  ERDI Gergo  <cactus cactus rulez org>
+
+	* em-folder-view.c (emfv_on_url_cb): Emit a hover-url signal when
+	the user mouses over a URL, ...
+	(emfv_hover_url_impl): ... and use BonoboUI to change the status
+	bar message...
+	(em_folder_view_set_statusbar): ... unless we are asked not to, ...
+
+	* mail-component.c (impl_createControls): ... like in the case of
+	the mail component, ...
+	(view_hover_url_cb): ... that uses the ActivityHandler to do the
+	same
+
+	Add these together, and #127536 is neatly solved.
+
+
 2003-12-03  Jeffrey Stedfast  <fejj ximian com>
 
 	* em-folder-view.c (emfv_set_folder): Sync the folder before
Index: mail/em-folder-view.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
retrieving revision 1.17
diff -u -r1.17 em-folder-view.c
--- mail/em-folder-view.c	3 Dec 2003 19:24:43 -0000	1.17
+++ mail/em-folder-view.c	4 Dec 2003 11:57:21 -0000
@@ -65,6 +65,7 @@
 #include "em-message-browser.h"
 #include "message-list.h"
 #include "em-utils.h"
+#include "em-marshal.h"
 
 #include <gtkhtml/gtkhtml.h>
 #include <gtkhtml/htmlobject.h>
@@ -102,6 +103,9 @@
 
 static void emfv_setting_setup(EMFolderView *emfv);
 
+static void emfv_on_url_cb(GObject *emitter, const char *url, EMFolderView *emfv);
+static void emfv_hover_url_impl(EMFolderView *emfv, const char *uri, const char *nice_uri);
+
 static const EMFolderViewEnable emfv_enable_map[];
 
 struct _EMFolderViewPrivate {
@@ -114,10 +118,19 @@
 
 	GtkWidget *invisible;
 	char *selection_uri;
+
+	gboolean do_statusbar;
 };
 
 static GtkVBoxClass *emfv_parent;
 
+enum {
+	HOVER_URL,
+	LAST_SIGNAL
+};
+
+static guint signals[LAST_SIGNAL] = { 0, };
+
 static void emfv_selection_get(GtkWidget *widget, GtkSelectionData *data, guint info, guint time_stamp, EMFolderView *emfv);
 static void emfv_selection_clear_event(GtkWidget *widget, GdkEventSelection *event, EMFolderView *emfv);
 
@@ -129,8 +142,10 @@
 
 	gtk_box_set_homogeneous (GTK_BOX (emfv), FALSE);
 
-	p = emfv->priv = g_malloc0(sizeof(struct _EMFolderViewPrivate));
+	p = emfv->priv = g_new0(struct _EMFolderViewPrivate, 1);
 
+	p->do_statusbar = TRUE;
+	
 	emfv->ui_files = g_slist_append(NULL, EVOLUTION_UIDIR "/evolution-mail-message.xml");
 	emfv->ui_app_name = "evolution-mail";
 
@@ -147,6 +162,7 @@
 	emfv->preview = (EMFormatHTMLDisplay *)em_format_html_display_new();
 	g_signal_connect(emfv->preview, "link_clicked", G_CALLBACK(emfv_format_link_clicked), emfv);
 	g_signal_connect(emfv->preview, "popup_event", G_CALLBACK(emfv_format_popup_event), emfv);
+	g_signal_connect (emfv->preview->formathtml.html, "on_url", G_CALLBACK (emfv_on_url_cb), emfv);

This should probably hang off emfv->preview via a signal proxied by EMFormatHTMLDisplay, the same way link_clicked is.  For consistency mostly. 
Remember consistent spacing too.
And consistency probably means you dont use _cb either :) 
 
 	p->invisible = gtk_invisible_new();
 	g_object_ref(p->invisible);
@@ -220,14 +236,27 @@
 static void
 emfv_class_init(GObjectClass *klass)
 {
+	EMFolderViewClass *emfv_class = (EMFolderViewClass*) klass;
+	
 	klass->finalize = emfv_finalise;
 	
 	((GtkObjectClass *) klass)->destroy = emfv_destroy;
 	
-	((EMFolderViewClass *)klass)->set_folder = emfv_set_folder;
-	((EMFolderViewClass *)klass)->set_folder_uri = emfv_set_folder_uri;
-	((EMFolderViewClass *)klass)->set_message = emfv_set_message;
-	((EMFolderViewClass *)klass)->activate = emfv_activate;
+	emfv_class->set_folder = emfv_set_folder;
+	emfv_class->set_folder_uri = emfv_set_folder_uri;
+	emfv_class->set_message = emfv_set_message;
+	emfv_class->activate = emfv_activate;
+
+	emfv_class->hover_url = emfv_hover_url_impl;

Please leave it how it was, using casts.

Also, dont use _impl for hover_url_impl, nothing else in this code does.

+	signals[HOVER_URL] = g_signal_new ("hover-url",

I think you should call it 'on-url', its really only proxying the gtkhtml event anyway.

Actually i dont understand why you are exporting this signal anyway?

You have two places the status is set, one from the signal callback in em-folder-view via the <status> tag, and the other from the callback in mail-component via activity client status.


+					   G_OBJECT_CLASS_TYPE (klass),
+					   G_SIGNAL_RUN_LAST,
+					   G_STRUCT_OFFSET (EMFolderViewClass, hover_url),
+					   NULL, NULL,
+					   em_marshal_VOID__STRING_STRING,
+					   G_TYPE_NONE,
+					   2, G_TYPE_STRING, G_TYPE_STRING);
 }
 
 GType
@@ -1547,6 +1576,8 @@
 		e_charset_picker_bonobo_ui_populate (uic, "/menu/View", _("Default"), emfv_charset_changed, emfv);
 
 		emfv_enable_menus(emfv);
+		if (emfv->priv->do_statusbar)
+			bonobo_ui_component_set_translate (uic, "/", "<status><item name=\"main\"/></status>", NULL);
 	} else {
 		const BonoboUIVerb *v;
 
@@ -1625,6 +1656,14 @@
 	return t;
 }
 
+void
+em_folder_view_set_statusbar (EMFolderView *emfv, gboolean statusbar)
+{
+	g_return_if_fail (emfv);
+	
+	emfv->priv->do_statusbar = statusbar;

if it was off, should it add it back on?  i'm not entirely clear why this is an option?

+}
+
 /* ********************************************************************** */
 
 struct mst_t {
@@ -2016,3 +2055,34 @@
 								emfv, NULL, NULL);
 	g_object_unref(gconf);
 }
+
+static void
+emfv_hover_url_impl (EMFolderView *emfv, const char *uri, const char *nice_uri)
+{
+	if (emfv->priv->do_statusbar)
+	{
+		if (emfv->uic) {
+			bonobo_ui_component_set_status (emfv->uic, nice_uri, NULL);

+			/* Make sure the node keeps existing if nice_url == 0 */
+			if (!nice_uri)
+				bonobo_ui_component_set_translate (
+					emfv->uic, "/", "<status><item name=\"main\"/></status>", NULL);

as i've statued beofre, i dont understand this logic.  why not set it to "" intsead of NULL, so you don't have to do this messy stuff?

+		}
+	}
+}
+
+static void
+emfv_on_url_cb (GObject *emitter, const char *url, EMFolderView *emfv)
+{

Why does this need to emit a signal, couldn't you just include the above code here?

+	char *nice_url = 0;
+
+	if (url)
+		if (strncmp (url, "mailto:", 7) == 0)
+			nice_url = g_strdup_printf (_("Click to mail %s"), url + 7);
+		else
+			nice_url = g_strdup_printf (_("Click to open %s"), url);
+
+	g_signal_emit (emfv, signals[HOVER_URL], 0, url, nice_url);
+	
+	g_free (nice_url);
+}
Index: mail/em-folder-view.h
===================================================================
RCS file: /cvs/gnome/evolution/mail/em-folder-view.h,v
retrieving revision 1.3
diff -u -r1.3 em-folder-view.h
--- mail/em-folder-view.h	19 Sep 2003 18:54:09 -0000	1.3
+++ mail/em-folder-view.h	4 Dec 2003 11:57:21 -0000
@@ -84,11 +84,13 @@
 	GtkVBoxClass parent_class;
 
 	/* if used as a control, used to activate/deactivate custom menu's */
-	void (*activate)(EMFolderView *, struct _BonoboUIComponent *uic, int state);
+	void (*activate)       (EMFolderView *, struct _BonoboUIComponent *uic, int state);
 
-	void (*set_folder_uri)(EMFolderView *emfv, const char *uri);
-	void (*set_folder)(EMFolderView *emfv, struct _CamelFolder *folder, const char *uri);
-	void (*set_message)(EMFolderView *emfv, const char *uid);
+	void (*set_folder_uri) (EMFolderView *emfv, const char *uri);
+	void (*set_folder)     (EMFolderView *emfv, struct _CamelFolder *folder, const char *uri);
+	void (*set_message)    (EMFolderView *emfv, const char *uid);
+
+	void (*hover_url)      (EMFolderView *emfv, const char *uri, const char *nice_uri);

i'd rather you didn't re-indent this stuff, but its not important.

put a comment in at least to say that hover_url is thte start of signals (the stuff above are virtula methods).
(and call it on_url :)

 };
 
 GType em_folder_view_get_type(void);
@@ -107,6 +109,8 @@
 
 int em_folder_view_print(EMFolderView *emfv, int preview);
 
+void em_folder_view_set_statusbar (EMFolderView *emfv, gboolean statusbar);
+
 /* this could be on message-list */
 guint32 em_folder_view_disable_mask(EMFolderView *emfv);
 
Index: mail/mail-component.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-component.c,v
retrieving revision 1.33
diff -u -r1.33 mail-component.c
--- mail/mail-component.c	2 Dec 2003 23:29:09 -0000	1.33
+++ mail/mail-component.c	4 Dec 2003 11:57:23 -0000
@@ -319,6 +319,12 @@
 	(* G_OBJECT_CLASS (parent_class)->finalize) (object);
 }
 
+static void
+view_hover_url_cb (GObject *emitter, const char *url, const char *nice_url, MailComponent *mail_component)
+{
+	MailComponentPrivate *priv = mail_component->priv;
+	e_activity_handler_message (priv->activity_handler, nice_url);
+}
 
 /* Evolution::Component CORBA methods.  */
 
@@ -335,10 +341,10 @@
 	BonoboControl *view_control;
 	BonoboControl *statusbar_control;
 	GtkWidget *tree_widget;
-	GtkWidget *view_widget;
+	EMFolderBrowser *view_widget;

Why change this type?

 	GtkWidget *statusbar_widget;
 	
-	view_widget = em_folder_browser_new ();
+	view_widget = (EMFolderBrowser*)em_folder_browser_new ();
 	tree_widget = (GtkWidget *) em_folder_tree_new_with_model (priv->model);
 	em_folder_tree_enable_drag_and_drop ((EMFolderTree *) tree_widget);
 	em_format_set_session ((EMFormat *) ((EMFolderView *) view_widget)->preview, session);
@@ -347,11 +353,11 @@
 	e_activity_handler_attach_task_bar (priv->activity_handler, E_TASK_BAR (statusbar_widget));
 
 	gtk_widget_show (tree_widget);
-	gtk_widget_show (view_widget);
+	gtk_widget_show (GTK_WIDGET (view_widget));

If you're going to cast, use (GtkWidget *)  (in the mailer at least - there are many lines of code adjacent to this one which dont use the gtk casts, so it should be obvious).  But you shouldn't need to change the type anyway.

 	gtk_widget_show (statusbar_widget);
 	
 	tree_control = bonobo_control_new (tree_widget);
-	view_control = bonobo_control_new (view_widget);
+	view_control = bonobo_control_new (GTK_WIDGET (view_widget));

 	statusbar_control = bonobo_control_new (statusbar_widget);
 	
 	*corba_tree_control = CORBA_Object_duplicate (BONOBO_OBJREF (tree_control), ev);
@@ -361,6 +367,9 @@
 	g_signal_connect (view_control, "activate", G_CALLBACK (view_control_activate_cb), view_widget);
 	
 	g_signal_connect (tree_widget, "folder-selected", G_CALLBACK (folder_selected_cb), view_widget);
+
+	g_signal_connect (view_widget, "hover-url", G_CALLBACK (view_hover_url_cb), mail_component);
+	em_folder_view_set_statusbar ((EMFolderView*)view_widget, FALSE);
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]