ayatana-commits team mailing list archive
-
ayatana-commits team
-
Mailing list archive
-
Message #02869
Re: lp:~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments into lp:indicator-datetime
Honestly, I don't see anything odd. I'm going to have to look closer
tomorrow. Especially nothing that would cause the craziness that you're
getting! Uhg...
On Wed, 2011-01-26 at 11:50 +0000, Karl Lattimer wrote:
> Karl Lattimer has proposed merging lp:~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments into lp:indicator-datetime.
>
> Requested reviews:
> Ted Gould (ted)
> Related bugs:
> #542218 integration with Evolution calendar events and time locations
> https://bugs.launchpad.net/bugs/542218
>
> For more details, see:
> https://code.launchpad.net/~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments/+merge/47514
>
> This change introduces a new dbusmenu item type of APPOINTMENT_MENUITEM_TYPE which has 3 columns of <image><appointment summary> -- <due time/date>
>
> with the new item type, we also introduce a small part of code to datetime-service which reads the EDS data for ecal and produces the values to fill into the menu items.
>
> However, there are issues with this.
>
> For some bizarre reason this indicator is affecting other indicators. Most notably the sound indicator which switches the volume up/down repeatedly, other indicators have strange effects too and it's difficult to see why this can happen in the code itself.
> differences between files attachment (review-diff.txt)
> === modified file 'configure.ac'
> --- configure.ac 2011-01-11 16:19:07 +0000
> +++ configure.ac 2011-01-26 11:50:11 +0000
> @@ -59,6 +59,8 @@
> INDICATOR_DISPLAY_OBJECTS=0.1.10
> GEOCLUE_REQUIRED_VERSION=0.12.0
> OOBS_REQUIRED_VERSION=2.31.0
> +ECAL_REQUIRED_VERSION=2.30
> +ICAL_REQUIRED_VERSION=0.44
>
> AS_IF([test "x$with_gtk" = x3],
> [PKG_CHECK_MODULES(INDICATOR, indicator3 >= $INDICATOR_REQUIRED_VERSION
> @@ -80,7 +82,9 @@
> libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS
> gio-2.0 >= $GIO_REQUIRED_VERSION
> geoclue >= $GEOCLUE_REQUIRED_VERSION
> - liboobs-1 >= $OOBS_REQUIRED_VERSION)
> + liboobs-1 >= $OOBS_REQUIRED_VERSION
> + libecal-1.2 >= $ECAL_REQUIRED_VERSION
> + libical >= $ICAL_REQUIRED_VERSION)
>
> AC_SUBST(INDICATOR_CFLAGS)
> AC_SUBST(INDICATOR_LIBS)
>
> === modified file 'src/datetime-service.c'
> --- src/datetime-service.c 2010-10-22 13:31:37 +0000
> +++ src/datetime-service.c 2011-01-26 11:50:11 +0000
> @@ -23,6 +23,7 @@
> #include <libindicator/indicator-service.h>
> #include <locale.h>
>
> +#include <gtk/gtk.h>
> #include <glib/gi18n.h>
> #include <gio/gio.h>
>
> @@ -33,6 +34,15 @@
> #include <geoclue/geoclue-master.h>
> #include <geoclue/geoclue-master-client.h>
>
> +#include <time.h>
> +#include <libecal/e-cal.h>
> +#include <libical/ical.h>
> +#include <libecal/e-cal-time-util.h>
> +#include <libedataserver/e-source.h>
> +// Other users of ecal seem to also include these, not sure why they should be included by the above
> +#include <libical/icaltime.h>
> +
> +
> #include <oobs/oobs-timeconfig.h>
>
> #include "datetime-interface.h"
> @@ -53,6 +63,9 @@
> static DbusmenuMenuitem * calendar = NULL;
> static DbusmenuMenuitem * settings = NULL;
> static DbusmenuMenuitem * tzchange = NULL;
> +static GList * appointments = NULL;
> +static ECal * ecal = NULL;
> +static const gchar * ecal_timezone = NULL;
>
> /* Geoclue trackers */
> static GeoclueMasterClient * geo_master = NULL;
> @@ -225,6 +238,186 @@
> return FALSE;
> }
>
> +static gboolean
> +update_timezone_menu_items(gpointer user_data) {
> + // Get the location preferences and the current location, highlight the current location somehow
> + return FALSE;
> +}
> +
> +/* Populate the menu with todays, next 5 appointments.
> + * we should hook into the ABOUT TO SHOW signal and use that to update the appointments.
> + * Experience has shown that caldav's and webcals can be slow to load from eds
> + * this is a problem mainly on the EDS side of things, not ours.
> + */
> +static gboolean
> +update_appointment_menu_items (gpointer user_data) {
> + // FFR: we should take into account short term timers, for instance
> + // tea timers, pomodoro timers etc... that people may add, this is hinted to in the spec.
> + time_t t1, t2;
> + icaltimezone *tzone;
> + gchar *query, *is, *ie;
> + GList *objects = NULL, *l;
> + GError *gerror = NULL;
> + DbusmenuMenuitem * item = NULL;
> + gint i;
> + gint width, height;
> +
> + if (!ecal)
> + ecal = e_cal_new_system_calendar();
> +
> + if (!ecal) {
> + g_debug("e_cal_new_system_calendar failed");
> + ecal = NULL;
> + return FALSE;
> + }
> + if (!e_cal_open(ecal, FALSE, &gerror) ) {
> + g_debug("e_cal_open: %s\n", gerror->message);
> + g_free(ecal);
> + ecal = NULL;
> + return FALSE;
> + }
> +
> + if (!e_cal_get_timezone(ecal, "UTC", &tzone, &gerror)) {
> + g_debug("failed to get time zone\n");
> + g_free(ecal);
> + ecal = NULL;
> + return FALSE;
> + }
> +
> + /* This timezone represents the timezone of the calendar, this might be different to the current UTC offset.
> + * this means we'll have some geoclue interaction going on, and possibly the user will be involved in setting
> + * their location manually, case in point: trains have satellite links which often geoclue to sweden,
> + * this shouldn't automatically set the location and mess up all the appointments for the user.
> + */
> + ecal_timezone = icaltimezone_get_tzid(tzone);
> +
> + time(&t1);
> + time(&t2);
> + t2 += (time_t) (7 * 24 * 60 * 60); /* 7 days ahead of now */
> +
> + is = isodate_from_time_t(t1);
> + ie = isodate_from_time_t(t2);
> + // FIXME can we put a limit on the number of results? Or if not complete, or is event/todo?
> + query = g_strdup_printf("(occur-in-time-range? (make-time\"%s\") (make-time\"%s\"))", is, ie);
> +
> + if (!e_cal_get_object_list_as_comp(ecal, query, &objects, &gerror)) {
> + g_debug("Failed to get objects\n");
> + g_free(ecal);
> + ecal = NULL;
> + return FALSE;
> + }
> + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
> + if (appointments != NULL) {
> + for (l = appointments; l; l = l->next) {
> + item = l->data;
> + // Remove all the existing menu items which are in appointments.
> + dbusmenu_menuitem_child_delete(root, DBUSMENU_MENUITEM(item));
> + appointments = g_list_remove(appointments, item);
> + g_free(item);
> + }
> + appointments = NULL;
> + }
> +
> + i = 0;
> + for (l = objects; l; l = l->next) {
> + ECalComponent *ecalcomp = l->data;
> + ECalComponentText valuetext;
> + ECalComponentDateTime datetime;
> + icaltimezone *appointment_zone = NULL;
> + icalproperty_status status;
> + gchar *summary, right[20], *cmd;
> + const gchar *uri;
> + struct tm tmp_tm;
> +
> + ECalComponentVType vtype = e_cal_component_get_vtype (ecalcomp);
> +
> + // See above FIXME regarding query result
> + // If it's not an event or todo, continue no-increment
> + if (vtype != E_CAL_COMPONENT_EVENT || vtype != E_CAL_COMPONENT_TODO)
> + continue;
> +
> + // See above FIXME regarding query result
> + // if the status is completed, continue no-increment
> + e_cal_component_get_status (ecalcomp, &status);
> + if (status == ICAL_STATUS_COMPLETED || status == ICAL_STATUS_CANCELLED)
> + continue;
> +
> + // INPROGRESS: Create a menu item for each of them, try to include helpful metadata e.g. colours, due time
> + item = dbusmenu_menuitem_new();
> + dbusmenu_menuitem_property_set (item, "type", APPOINTMENT_MENUITEM_TYPE);
> + dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_ENABLED, TRUE);
> + dbusmenu_menuitem_property_set_bool (item, DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE);
> +
> + // Label text
> + e_cal_component_get_summary (ecalcomp, &valuetext);
> + summary = g_strdup (valuetext.value);
> +
> + dbusmenu_menuitem_property_set (item, APPOINTMENT_MENUITEM_PROP_LABEL, summary);
> +
> + g_free (summary);
> +
> + // Due text
> + if (vtype == E_CAL_COMPONENT_EVENT)
> + e_cal_component_get_dtstart (ecalcomp, &datetime);
> + else
> + e_cal_component_get_due (ecalcomp, &datetime);
> +
> + // FIXME need to get the timezone of the above datetime,
> + // and get the icaltimezone of the geoclue timezone/selected timezone (whichever is preferred)
> + if (!datetime.value) {
> + g_free(item);
> + continue;
> + }
> +
> + if (!appointment_zone || datetime.value->is_date) { // If it's today put in the current timezone?
> + appointment_zone = tzone;
> + }
> + tmp_tm = icaltimetype_to_tm_with_zone (datetime.value, appointment_zone, tzone);
> +
> + e_cal_component_free_datetime (&datetime);
> +
> + // Get today
> + if (datetime.value->is_date)
> + strftime(right, sizeof(right), "%X", &tmp_tm);
> + else
> + strftime(right, sizeof(right), "%a %X", &tmp_tm);
> + dbusmenu_menuitem_property_set (item, APPOINTMENT_MENUITEM_PROP_RIGHT, right);
> +
> + // Now we pull out the URI for the calendar event and try to create a URI that'll work when we execute evolution
> + e_cal_component_get_url(ecalcomp, &uri);
> + cmd = g_strconcat("evolution ", uri, NULL);
> + g_signal_connect (G_OBJECT(item), DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
> + G_CALLBACK (activate_cb), cmd);
> +
> + // Get the colour E_CAL_COMPONENT_FIELD_COLOR
> + // Get the icon, either EVENT or MEMO or TODO?
> + /*gdouble red, blue, green;
> + ECalSource *source = e_cal_get_source (ecalcomp->client);
> + if (!ecalcomp->color && e_source_get_color (source, &source_color)) {
> + g_free (comp_data->color);
> + ecalcomp->color = g_strdup_printf ("#%06x", source_color & 0xffffff);
> + }*/
> +
> +
> + //cairo_surface_t *cs = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, width, height);
> + //cairo_t *cr = cairo_create(cs);
> +
> + // TODO: Draw the correct icon for the appointment type and then tint it using mask fill.
> +
> + //GdkPixbuf * pixbuf = gdk_pixbuf_get_from_drawable(NULL, (GdkDrawable*)cs, 0,0,0,0, width, height);
> +
> + //dbusmenu_menuitem_property_set_image (item, APPOINTMENT_MENUITEM_PROP_ICON, pixbuf);
> +
> + dbusmenu_menuitem_child_append (root, item);
> + appointments = g_list_append (appointments, item); // Keep track of the items here to make them east to remove
> +
> + if (i == 4) break; // See above FIXME regarding query result limit
> + i++;
> + }
> + g_free(ecal); // Really we should do the setup somewhere where we know it'll only run once, right now, we'll do it every time and free it.
> + return TRUE;
> +}
> +
> /* Looks for the time and date admin application and enables the
> item we have one */
> static gboolean
> @@ -272,8 +465,28 @@
>
> g_idle_add(check_for_calendar, NULL);
> }
> + DbusmenuMenuitem * separator;
> +
> + separator = dbusmenu_menuitem_new();
> + dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, DBUSMENU_CLIENT_TYPES_SEPARATOR);
> + dbusmenu_menuitem_child_append(root, separator);
>
> - DbusmenuMenuitem * separator = dbusmenu_menuitem_new();
> + // This just populates the items on startup later we want to be able to update the appointments before
> + // presenting the menu.
> + update_appointment_menu_items(NULL);
> + if (calendar != NULL) {
> + // TODO Create "Add appointment" menu item
> + }
> + // TODO Create FFR? "Add timer" menu item
> +
> + separator = dbusmenu_menuitem_new();
> + dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, DBUSMENU_CLIENT_TYPES_SEPARATOR);
> + dbusmenu_menuitem_child_append(root, separator);
> +
> + update_timezone_menu_items(NULL);
> + // TODO Create "Add location" menu item
> +
> + separator = dbusmenu_menuitem_new();
> dbusmenu_menuitem_property_set(separator, DBUSMENU_MENUITEM_PROP_TYPE, DBUSMENU_CLIENT_TYPES_SEPARATOR);
> dbusmenu_menuitem_child_append(root, separator);
>
>
> === modified file 'src/dbus-shared.h'
> --- src/dbus-shared.h 2010-08-31 12:57:01 +0000
> +++ src/dbus-shared.h 2011-01-26 11:50:11 +0000
> @@ -29,3 +29,12 @@
>
> #define DBUSMENU_CALENDAR_MENUITEM_TYPE "x-canonical-calendar-item"
>
> +#define APPOINTMENT_MENUITEM_TYPE "appointment-item"
> +#define APPOINTMENT_MENUITEM_PROP_LABEL "appointment-label"
> +#define APPOINTMENT_MENUITEM_PROP_ICON "appointment-icon"
> +#define APPOINTMENT_MENUITEM_PROP_RIGHT "appointment-time"
> +
> +#define TIMEZONE_MENUITEM_TYPE "timezone-item"
> +#define TIMEZONE_MENUITEM_PROP_LABEL "timezone-label"
> +#define TIMEZONE_MENUITEM_PROP_RADIO "timezone-radio"
> +#define TIMEZONE_MENUITEM_PROP_RIGHT "timezone-time"
>
> === modified file 'src/indicator-datetime.c'
> --- src/indicator-datetime.c 2011-01-14 06:19:05 +0000
> +++ src/indicator-datetime.c 2011-01-26 11:50:11 +0000
> @@ -41,6 +41,7 @@
> /* DBusMenu */
> #include <libdbusmenu-gtk/menu.h>
> #include <libido/idocalendarmenuitem.h>
> +#include <libdbusmenu-gtk/menuitem.h>
>
> #include "dbus-shared.h"
>
> @@ -102,6 +103,13 @@
> PROP_CUSTOM_TIME_FORMAT
> };
>
> +typedef struct _indicator_item_t indicator_item_t;
> +struct _indicator_item_t {
> + GtkWidget * icon;
> + GtkWidget * label;
> + GtkWidget * right;
> +};
> +
> #define PROP_TIME_FORMAT_S "time-format"
> #define PROP_SHOW_SECONDS_S "show-seconds"
> #define PROP_SHOW_DAY_S "show-day"
> @@ -174,6 +182,8 @@
>
> G_DEFINE_TYPE (IndicatorDatetime, indicator_datetime, INDICATOR_OBJECT_TYPE);
>
> +static GtkSizeGroup * indicator_right_group = NULL;
> +
> static void
> indicator_datetime_class_init (IndicatorDatetimeClass *klass)
> {
> @@ -1055,6 +1065,128 @@
> return g_strdup_printf(T_("%s, %s"), date_string, time_string);
> }
>
> +/* Whenever we have a property change on a DbusmenuMenuitem
> + we need to be responsive to that. */
> +static void
> +indicator_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, gchar * value, indicator_item_t * mi_data)
> +{
> + if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_LABEL)) {
> + /* Set the main label */
> + gtk_label_set_text(GTK_LABEL(mi_data->label), value);
> + } else if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_RIGHT)) {
> + /* Set the right label */
> + gtk_label_set_text(GTK_LABEL(mi_data->right), value);
> + } else if (!g_strcmp0(prop, APPOINTMENT_MENUITEM_PROP_ICON)) {
> + /* We don't use the value here, which is probably less efficient,
> + but it's easier to use the easy function. And since th value
> + is already cached, shouldn't be a big deal really. */
> + GdkPixbuf * pixbuf = dbusmenu_menuitem_property_get_image(mi, APPOINTMENT_MENUITEM_PROP_ICON);
> + if (pixbuf != NULL) {
> + /* If we've got a pixbuf we need to make sure it's of a reasonable
> + size to fit in the menu. If not, rescale it. */
> + GdkPixbuf * resized_pixbuf;
> + gint width, height;
> + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
> + if (gdk_pixbuf_get_width(pixbuf) > width ||
> + gdk_pixbuf_get_height(pixbuf) > height) {
> + g_debug("Resizing icon from %dx%d to %dx%d", gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf), width, height);
> + resized_pixbuf = gdk_pixbuf_scale_simple(pixbuf,
> + width,
> + height,
> + GDK_INTERP_BILINEAR);
> + } else {
> + g_debug("Happy with icon sized %dx%d", gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf));
> + resized_pixbuf = pixbuf;
> + }
> + gtk_image_set_from_pixbuf(GTK_IMAGE(mi_data->icon), resized_pixbuf);
> + /* The other pixbuf should be free'd by the dbusmenu. */
> + if (resized_pixbuf != pixbuf) {
> + g_object_unref(resized_pixbuf);
> + }
> + }
> + } else {
> + g_warning("Indicator Item property '%s' unknown", prop);
> + }
> + return;
> +}
> +
> +/* We have a small little menuitem type that handles all
> + of the fun stuff for indicators. Mostly this is the
> + shifting over and putting the icon in with some right
> + side text that'll be determined by the service.
> + Copied verbatim from an old revision (including comments) of indicator-messages
> +*/
> +static gboolean
> +new_appointment_item (DbusmenuMenuitem * newitem, DbusmenuMenuitem * parent, DbusmenuClient * client)
> +{
> + g_return_val_if_fail(DBUSMENU_IS_MENUITEM(newitem), FALSE);
> + g_return_val_if_fail(DBUSMENU_IS_GTKCLIENT(client), FALSE);
> + /* Note: not checking parent, it's reasonable for it to be NULL */
> +
> + indicator_item_t * mi_data = g_new0(indicator_item_t, 1);
> +
> + GtkMenuItem * gmi = GTK_MENU_ITEM(gtk_menu_item_new());
> +
> + GtkWidget * hbox = gtk_hbox_new(FALSE, 4);
> +
> + /* Icon, probably someone's face or avatar on an IM */
> + mi_data->icon = gtk_image_new();
> + GdkPixbuf * pixbuf = dbusmenu_menuitem_property_get_image(newitem, APPOINTMENT_MENUITEM_PROP_ICON);
> +
> + if (pixbuf != NULL) {
> + /* If we've got a pixbuf we need to make sure it's of a reasonable
> + size to fit in the menu. If not, rescale it. */
> + GdkPixbuf * resized_pixbuf;
> + gint width, height;
> + gtk_icon_size_lookup(GTK_ICON_SIZE_MENU, &width, &height);
> + if (gdk_pixbuf_get_width(pixbuf) > width ||
> + gdk_pixbuf_get_height(pixbuf) > height) {
> + g_debug("Resizing icon from %dx%d to %dx%d", gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf), width, height);
> + resized_pixbuf = gdk_pixbuf_scale_simple(pixbuf,
> + width,
> + height,
> + GDK_INTERP_BILINEAR);
> + } else {
> + g_debug("Happy with icon sized %dx%d", gdk_pixbuf_get_width(pixbuf), gdk_pixbuf_get_height(pixbuf));
> + resized_pixbuf = pixbuf;
> + }
> +
> + gtk_image_set_from_pixbuf(GTK_IMAGE(mi_data->icon), resized_pixbuf);
> +
> + /* The other pixbuf should be free'd by the dbusmenu. */
> + if (resized_pixbuf != pixbuf) {
> + g_object_unref(resized_pixbuf);
> + }
> + }
> + gtk_misc_set_alignment(GTK_MISC(mi_data->icon), 0.0, 0.5);
> + gtk_box_pack_start(GTK_BOX(hbox), mi_data->icon, FALSE, FALSE, 0);
> + gtk_widget_show(mi_data->icon);
> +
> + /* Label, probably a username, chat room or mailbox name */
> + mi_data->label = gtk_label_new(dbusmenu_menuitem_property_get(newitem, APPOINTMENT_MENUITEM_PROP_LABEL));
> + gtk_misc_set_alignment(GTK_MISC(mi_data->label), 0.0, 0.5);
> + gtk_box_pack_start(GTK_BOX(hbox), mi_data->label, TRUE, TRUE, 0);
> + gtk_widget_show(mi_data->label);
> +
> + /* Usually either the time or the count on the individual
> + item. */
> + mi_data->right = gtk_label_new(dbusmenu_menuitem_property_get(newitem, APPOINTMENT_MENUITEM_PROP_RIGHT));
> + gtk_size_group_add_widget(indicator_right_group, mi_data->right);
> + gtk_misc_set_alignment(GTK_MISC(mi_data->right), 1.0, 0.5);
> + gtk_box_pack_start(GTK_BOX(hbox), mi_data->right, FALSE, FALSE, 0);
> + gtk_widget_show(mi_data->right);
> +
> + gtk_container_add(GTK_CONTAINER(gmi), hbox);
> + gtk_widget_show(hbox);
> +
> + dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, gmi, parent);
> +
> + g_signal_connect(G_OBJECT(newitem), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK(indicator_prop_change_cb), mi_data);
> + g_signal_connect(G_OBJECT(newitem), "destroyed", G_CALLBACK(g_free), mi_data);
> +
> + return TRUE;
> +}
> +
> static gboolean
> new_calendar_item (DbusmenuMenuitem * newitem,
> DbusmenuMenuitem * parent,
> @@ -1081,6 +1213,16 @@
> return TRUE;
> }
>
> +static gboolean
> +new_timezone_item(DbusmenuMenuitem * newitem,
> + DbusmenuMenuitem * parent,
> + DbusmenuClient * client)
> +{
> + // Menu item with a radio button and a right aligned time
> +
> + return TRUE;
> +}
> +
> /* Grabs the label. Creates it if it doesn't
> exist already */
> static GtkLabel *
> @@ -1120,6 +1262,8 @@
> g_object_set_data (G_OBJECT (client), "indicator", io);
>
> dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_CALENDAR_MENUITEM_TYPE, new_calendar_item);
> + dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), APPOINTMENT_MENUITEM_TYPE, new_appointment_item);
> + dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), TIMEZONE_MENUITEM_TYPE, new_timezone_item);
>
> return GTK_MENU(self->priv->menu);
> }
>
--
https://code.launchpad.net/~karl-qdh/ubuntu/natty/indicator-datetime/indicator-datetime.withappointments/+merge/47514
Your team ayatana-commits is subscribed to branch lp:indicator-datetime.
References
