Merge lp:~larsu/ido/lp733285 into lp:ido/14.04

[Lars Karlitski]

Highlight back/forward buttons when hovering them with the pointer

Space bar still activates play/pause, unless the mouse pointer hovers another button.

Highlight back/forward buttons when hovering them with the pointer

Space bar still activates play/pause, unless the mouse pointer hovers another button.

[Sebastien Bacher (seb128)]

I didn't review the code but the changes works fine from an user perspective ;-)

review: Approve (user runtime testing)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:177
http://jenkins.qa.ubuntu.com/job/ido-ci/58/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ido-trusty-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ido-trusty-armhf-ci/16
        deb: http://jenkins.qa.ubuntu.com/job/ido-trusty-armhf-ci/16/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ido-ci/58/rebuild

review: Approve (continuous-integration)

=== modified file 'src/idoplaybackmenuitem.c'
--- src/idoplaybackmenuitem.c	2014-03-24 15:09:02 +0000
+++ src/idoplaybackmenuitem.c	2014-03-25 18:04:35 +0000
@@ -39,7 +39,8 @@
   BUTTON_NONE,
   BUTTON_PREVIOUS,
   BUTTON_PLAYPAUSE,
-  BUTTON_NEXT
+  BUTTON_NEXT,
+  N_BUTTONS
 } Button;
 
 typedef GtkMenuItemClass IdoPlaybackMenuItemClass;
@@ -55,9 +56,7 @@
   gboolean keyboard_activated; /* TRUE if the current button was activated with a key */
 
   GActionGroup *action_group;
-  gchar *play_action;
-  gchar *next_action;
-  gchar *prev_action;
+  gchar *button_actions[N_BUTTONS];
 };
 
 G_DEFINE_TYPE (IdoPlaybackMenuItem, ido_playback_menu_item, GTK_TYPE_MENU_ITEM);
@@ -82,10 +81,10 @@
 ido_playback_menu_item_finalize (GObject *object)
 {
   IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (object);
+  gint i;
 
-  g_free (item->play_action);
-  g_free (item->next_action);
-  g_free (item->prev_action);
+  for (i = 0; i < N_BUTTONS; i++)
+    g_free (item->button_actions[i]);
 
   G_OBJECT_CLASS (ido_playback_menu_item_parent_class)->finalize (object);
 }
@@ -129,20 +128,17 @@
     {
     case GDK_KEY_Left:
       self->cur_pushed_button = BUTTON_PREVIOUS;
-      if (self->action_group && self->prev_action)
-        g_action_group_activate_action (self->action_group, self->prev_action, NULL);
       break;
 
     case GDK_KEY_Right:
       self->cur_pushed_button = BUTTON_NEXT;
-      if (self->action_group && self->next_action)
-        g_action_group_activate_action (self->action_group, self->next_action, NULL);
       break;
 
     case GDK_KEY_space:
-      self->cur_pushed_button = BUTTON_PLAYPAUSE;
-      if (self->action_group && self->play_action)
-        g_action_group_activate_action (self->action_group, self->play_action, NULL);
+      if (self->cur_hover_button != BUTTON_NONE)
+        self->cur_pushed_button = self->cur_hover_button;
+      else
+        self->cur_pushed_button = BUTTON_PLAYPAUSE;
       break;
 
     default:
@@ -151,6 +147,11 @@
 
   if (self->cur_pushed_button != BUTTON_NONE)
     {
+      const gchar *action = self->button_actions[self->cur_pushed_button];
+
+      if (self->action_group && action)
+        g_action_group_activate_action (self->action_group, action, NULL);
+
       self->keyboard_activated = TRUE;
       gtk_widget_queue_draw (widget);
       return TRUE;
@@ -243,31 +244,15 @@
 {
   IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (menuitem);
   Button button;
+  const gchar *action = action;
 
   button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
   if (button != item->cur_pushed_button)
     button = BUTTON_NONE;
 
-  switch (button)
-    {
-    case BUTTON_NONE:
-      break;
-
-    case BUTTON_PREVIOUS:
-      if (item->action_group && item->prev_action)
-        g_action_group_activate_action (item->action_group, item->prev_action, NULL);
-      break;
-
-    case BUTTON_NEXT:
-      if (item->action_group && item->next_action)
-        g_action_group_activate_action (item->action_group, item->next_action, NULL);
-      break;
-
-    case BUTTON_PLAYPAUSE:
-      if (item->action_group && item->play_action)
-        g_action_group_activate_action (item->action_group, item->play_action, NULL);
-      break;
-    }
+  action = item->button_actions[item->cur_pushed_button];
+  if (item->action_group && action)
+    g_action_group_activate_action (item->action_group, action, NULL);
 
   item->cur_pushed_button = BUTTON_NONE;
   gtk_widget_queue_draw (menuitem);
@@ -363,12 +348,14 @@
                                      gpointer      user_data)
 {
   IdoPlaybackMenuItem *self = user_data;
+  const gchar *action;
 
-  if (self->play_action && g_str_equal (action_name, self->play_action))
+  action = self->button_actions[BUTTON_PLAYPAUSE];
+  if (action && g_str_equal (action_name, action))
     {
       GVariant *state;
 
-      state = g_action_group_get_action_state (action_group, self->play_action);
+      state = g_action_group_get_action_state (action_group, action);
       if (g_variant_is_of_type (state, G_VARIANT_TYPE_STRING))
         ido_playback_menu_item_set_state_from_string (self, g_variant_get_string (state, NULL));
 
@@ -382,8 +369,10 @@
                                        gpointer      user_data)
 {
   IdoPlaybackMenuItem *self = user_data;
+  const gchar *action;
 
-  if (self->play_action && g_str_equal (action_name, self->play_action))
+  action = self->button_actions[BUTTON_PLAYPAUSE];
+  if (action && g_str_equal (action_name, action))
     ido_playback_menu_item_set_state (self, STATE_PAUSED);
 }
 
@@ -394,10 +383,13 @@
                                              gpointer      user_data)
 {
   IdoPlaybackMenuItem *self = user_data;
+  const gchar *action;
 
   g_return_if_fail (action_name != NULL);
 
-  if (self->play_action && g_str_equal (action_name, self->play_action))
+  action = self->button_actions[BUTTON_PLAYPAUSE];
+
+  if (action && g_str_equal (action_name, action))
     {
       if (g_variant_is_of_type (value, G_VARIANT_TYPE_STRING))
         ido_playback_menu_item_set_state_from_string (self, g_variant_get_string (value, NULL));
@@ -409,6 +401,7 @@
                                        GActionGroup *actions)
 {
   IdoPlaybackMenuItem *widget;
+  gchar *play_action;
 
   widget = g_object_new (IDO_TYPE_PLAYBACK_MENU_ITEM, NULL);
 
@@ -417,12 +410,13 @@
   g_signal_connect (actions, "action-added", G_CALLBACK (ido_playback_menu_item_action_added), widget);
   g_signal_connect (actions, "action-removed", G_CALLBACK (ido_playback_menu_item_action_removed), widget);
 
-  g_menu_item_get_attribute (item, "x-canonical-play-action", "s", &widget->play_action);
-  g_menu_item_get_attribute (item, "x-canonical-next-action", "s", &widget->next_action);
-  g_menu_item_get_attribute (item, "x-canonical-previous-action", "s", &widget->prev_action);
+  g_menu_item_get_attribute (item, "x-canonical-play-action", "s", &widget->button_actions[BUTTON_PLAYPAUSE]);
+  g_menu_item_get_attribute (item, "x-canonical-next-action", "s", &widget->button_actions[BUTTON_NEXT]);
+  g_menu_item_get_attribute (item, "x-canonical-previous-action", "s", &widget->button_actions[BUTTON_PREVIOUS]);
 
-  if (widget->play_action && g_action_group_has_action (actions, widget->play_action))
-    ido_playback_menu_item_action_added (actions, widget->play_action, widget);
+  play_action = widget->button_actions[BUTTON_PLAYPAUSE];
+  if (play_action && g_action_group_has_action (actions, play_action))
+    ido_playback_menu_item_action_added (actions, play_action, widget);
 
   return GTK_MENU_ITEM (widget);
 }
@@ -1434,7 +1428,8 @@
   }
 
   // draw previous-button drop-shadow
-  if (item->cur_pushed_button == BUTTON_PREVIOUS && item->keyboard_activated )
+  if ((item->cur_pushed_button == BUTTON_PREVIOUS && item->keyboard_activated) ||
+      item->cur_hover_button == BUTTON_PREVIOUS)
   {
     _setup (&cr_surf, &surf, PREV_WIDTH+6, PREV_HEIGHT+6);
     _mask_prev (cr_surf,
@@ -1494,7 +1489,8 @@
   _finalize (cr, &cr_surf, &surf, PREV_X, PREV_Y);
 
   // draw next-button drop-shadow
-  if (item->cur_pushed_button == BUTTON_NEXT && item->keyboard_activated)
+  if ((item->cur_pushed_button == BUTTON_NEXT && item->keyboard_activated) ||
+      item->cur_hover_button == BUTTON_NEXT)
   {
     _setup (&cr_surf, &surf, NEXT_WIDTH+6, NEXT_HEIGHT+6);
     _mask_next (cr_surf,
@@ -1557,6 +1553,7 @@
   if (item->current_state == STATE_PLAYING)
   {
     if (item->has_focus &&
+        (item->cur_hover_button == BUTTON_NONE || item->cur_hover_button == BUTTON_PLAYPAUSE) &&
         (item->cur_pushed_button == BUTTON_NONE || item->cur_pushed_button == BUTTON_PLAYPAUSE))
     {
       _setup (&cr_surf, &surf, PAUSE_WIDTH+6, PAUSE_HEIGHT+6);
@@ -1619,6 +1616,7 @@
   else if (item->current_state == STATE_PAUSED)
   {
     if (item->has_focus &&
+        (item->cur_hover_button == BUTTON_NONE || item->cur_hover_button == BUTTON_PLAYPAUSE) &&
         (item->cur_pushed_button == BUTTON_NONE || item->cur_pushed_button == BUTTON_PLAYPAUSE))
     {
       _setup (&cr_surf, &surf, PLAY_WIDTH+6, PLAY_HEIGHT+6);