Created attachment 344649 [details] [review]
Expand events details with a tooltip when the mouse is hovering over the event.

In month view and year view, when the mouse is hovering over an event a GtkTooltip now appears which says the full title, the location and notes of the event.

Review of attachment 344649 [details] [review]:

Thanks for the patch. Overall, the patch has a triple outstanding issues:

 - It doesn't strictly follow the GNU C Coding Standard, and is mixing variable declarations with function body
 - The strings aren't translatable
 - The tooltip is plain text. I'd expect bold being used more here.

I think the tooltip can be structured as follows (thanks for the initial suggestion Jean!):

'''
<b> [event title] </b>
[start time] - [end time] <--- must check if the event is a single day, full day event

At [location] <--- Only if it has a location

Event notes
'''

::: src/gcal-event-widget.c
@@ +382,3 @@
   gdk_window_set_cursor (self->event_window, pointer_cursor);
+    
+  gchar *tooltip_mesg = g_strjoin (NULL, 

Don't mix variable declarations with function body.

@@ +383,3 @@
+    
+  gchar *tooltip_mesg = g_strjoin (NULL, 
+                                   "Title: ", 

Not translatable.

@@ +390,3 @@
+                                   gcal_event_get_description (self->event), 
+                                   NULL);
+  gtk_widget_set_tooltip_text (widget, tooltip_mesg);;

gtk_widget_set_tooltip_markup();

Also, double ';'

Created attachment 347680 [details] [review]
Add a tooltip with event details on hover

Show a markup formatted tooltip on hovering over an event. Add a title, time/date, location and notes in the tooltip. Don't show location and/or notes if not present.

Review of attachment 347680 [details] [review]:

gcal_event_widget_realize() is not the correct place to write this code. The correct place would be gcal_event_widget_set_event_internal().

Created attachment 348329 [details] [review]
Expand events details with a tooltip when the mouse is hovering over the event.

In month view and year view, when the mouse is hovering over an event a GtkTooltip now appears which displays the event's full title, time/date, and if present, the location and notes of the event. Long notes are truncated at a white space and ellipsized.

Review of attachment 348329 [details] [review]:

Needs some work still. Also, this whole code should be factored in a separate function, so that whenever the event changes title, comments or dates, we update the tooltip.

::: src/gcal-event-widget.c
@@ +191,3 @@
+  /* Expand event details with a tooltip */
+  gchar *tooltip_mesg, *tooltip_desc;
+  GDateTime *tooltip_start, *tooltip_end;

This should be at the top of the function, sorted by the type name length.

I think using a GString, rather than plain gchars, will be a better choice here.

Finally, by using g_autoptr you won't need to unref() these vars later.

@@ +206,3 @@
+    {
+      tooltip_mesg = g_strjoin (NULL, tooltip_mesg, "\n", 
+                                g_date_time_format (tooltip_start, "%x"), NULL);

One parameter per line.

@@ +212,3 @@
+      tooltip_mesg = g_strjoin (NULL, tooltip_mesg, "\n", 
+                                g_date_time_format (tooltip_start, "%H:%M"),
+                                " - ", g_date_time_format (tooltip_end, "%H:%M"), NULL);

Ditto

@@ +221,3 @@
+    {
+      tooltip_mesg = g_strjoin (NULL, tooltip_mesg, "\n", _("At"), " ", 
+                                gcal_event_get_location (event), NULL);

Ditto

@@ +227,3 @@
+  if (tooltip_desc != "")
+    {
+      gint desc_max_char = 200;

This should be a #define at the top of the file.

@@ +231,3 @@
+      
+      /* Truncate long descriptions at a white space and ellipsize */
+      if (strlen(tooltip_desc) > desc_max_char)

Use g_utf8_strlen(). Also, there's a style problem here.

@@ +233,3 @@
+      if (strlen(tooltip_desc) > desc_max_char)
+        {
+          gint k = 0;

First declarations, then assignments.

@@ +234,3 @@
+        {
+          gint k = 0;
+          p = tooltip_desc + desc_max_char;

If you're using 'p' here, declare it inside the if().

Jump a line below.

@@ +235,3 @@
+          gint k = 0;
+          p = tooltip_desc + desc_max_char;
+          while (!g_ascii_isspace(*p))

Wront style.

@@ +240,3 @@
+              ++k; 
+            }
+          tooltip_desc = g_strndup (tooltip_desc, desc_max_char - k);

Jump a line above.

Created attachment 348723 [details] [review]
Expand events details with a tooltip when the mouse is hovering over the event.

In month view and year view, when the mouse is hovering over an event a GtkTooltip now appears which displays the event's full title, time/date, and if present, the location and notes of the event. Long notes are truncated at a white space and ellipsized.

Review of attachment 348723 [details] [review]:

It's getting close! I propose you to check GcalQuickAddPopover to see how the start/end dates are treated.

Basically, it should be something among the following lines:

 - All day, single day: "%x"
 - All day, multiday: "%x - %x"
 - Timed, single day: "%x, %X - %X"
 - Timed, multiday: "%x %X - %x %X"

This is just the general idea. Check GcalQuickAddPopover, it contains a reference implementation.

::: src/gcal-event-widget.c
@@ +250,3 @@
+  GString *tooltip_mesg;
+  g_autoptr (GDateTime) tooltip_end;
+  g_autoptr (GDateTime) tooltip_start;

Nitpick... this should be:

g_autoptr (GDateTime) tooltip_start, tooltip_end;
GString *tooltip_desc, *tooltip_mesg;

@@ +258,3 @@
+  tooltip_end = g_date_time_to_local (gcal_event_get_date_end (event));
+
+  if (gcal_event_get_all_day (event))

This check here is wrong, for the event can be all day AND multiday.

@@ +262,3 @@
+      g_string_append_printf (tooltip_mesg,
+                              "\n%s",
+                              g_date_time_format (tooltip_start, "%x"));

Memory leak. g_date_time_format() needs to be free()d after use.

@@ +269,3 @@
+                              "\n%s - %s",
+                              g_date_time_format (tooltip_start, "%H:%M"),
+                              g_date_time_format (tooltip_end, "%H:%M"));

Ditto

@@ +272,3 @@
+    }
+
+  if (gcal_event_get_location (event) != "")

Use g_utf8_strlen()

@@ +277,3 @@
+                              "\n%s %s",
+                              _("At"),
+                              gcal_event_get_location (event));

Translators wouldn't be able to control the position of the 'At'. Not every language works like English :)

The correct way is to make "At %s" translatable. It means that you should remove the '\n' from this string (put it in a separate g_string_append() call).

@@ +301,3 @@
+          tooltip_desc = g_string_truncate (tooltip_desc, DESC_MAX_CHAR - k);
+          tooltip_desc = g_string_append (tooltip_desc, "...");
+        }

If you use the '…' char, this whole condition could be simple as:

g_string_truncate (str, DESC_MAX_CAR - 1);
g_string_append (str, "…");

@@ +306,3 @@
+    }
+
+  gtk_widget_set_tooltip_markup (self, tooltip_mesg->str);

This introduces a warning. The first argument should be a GtkWidget.

Created attachment 348755 [details] [review]
Expand events details with a tooltip when the mouse is hovering over the event.

In month view and year view, when the mouse is hovering over an event a GtkTooltip now appears which displays the event's full title, time/date, and if present, the location and notes of the event. Long notes are truncated at a white space and ellipsized.

Created attachment 349670 [details] [review]
(Reattaching with text direction code, and a rewritten commit message)

event-widget: show tooltip with event details

In month view and year view, when the mouse is hovering over
an event, nothing is shown. A tooltip with the event information
would be much more useful in this scenario.

Fix that by showing a tooltip displaying the event's title, time
and date, and, if present, the location and notes of the event. Long
notes are truncated and ellipsized.

I took the liberty to improve the patch. Notice that I also rewrote the commit message, for the previous one wasn't following GNOME Calendar style, nor what's proposed at the GNOME Newcomers guide (https://wiki.gnome.org/Newcomers/SubmitPatch)

Thanks for working on this issue.

Pushed to master.

The new string could use a translator comment — it’s not immediately obvious how it should be translated.

https://wiki.gnome.org/TranslationProject/DevGuidelines/Use%20comments