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

[Lars Karlitski]

Make long track infos better readable

By (a) making the width of the menu slightly larger and (b) reducing the font size.

Also removes the hard-coded width of 200px in favor of a width based on font size (25 characters) and center the playback widget horizontally.

Make long track infos better readable

By (a) making the width of the menu slightly larger and (b) reducing the font size.

Also removes the hard-coded width of 200px in favor of a width based on font size (25 characters) and center the playback widget horizontally.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

A good improvement.

One thing I'd like to see is for the new numbers 41, 2, and 76 introduced
in ido_playback_menu_item_draw() to be #defined instead of being raw magic
numbers.

I realize idomediaplayermenuitem.c is already a mess and don't have
any illusions about this making the code right. Still, the signposting will
be useful to anyone who has to work this code in the future.

review: Needs Fixing

[lp:~larsu/ido/lp1080076]

186. By Lars Karlitski on 2014-04-23

idoplaybackmenuitem: don't introduce even more magic numbers

[Charles Kerr (charlesk)]

Thanks :)

review: Approve

=== modified file 'src/idomediaplayermenuitem.c'
--- src/idomediaplayermenuitem.c	2013-06-19 21:07:59 +0000
+++ src/idomediaplayermenuitem.c	2014-04-23 14:37:56 +0000
@@ -102,14 +102,6 @@
 }
 
 static void
-ido_media_player_menu_item_get_preferred_width (GtkWidget *widget,
-                                                gint      *minimum,
-                                                gint      *natural)
-{
-  *minimum = *natural = 200;
-}
-
-static void
 ido_media_player_menu_item_class_init (IdoMediaPlayerMenuItemClass *klass)
 {
   GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -117,10 +109,24 @@
 
   object_class->dispose = ido_media_player_menu_item_dispose;
 
-  widget_class->get_preferred_width = ido_media_player_menu_item_get_preferred_width;
   widget_class->draw = ido_media_player_menu_item_draw;
 }
 
+static GtkWidget *
+track_info_label_new ()
+{
+  GtkWidget *label;
+
+  label = gtk_label_new (NULL);
+  gtk_label_set_width_chars (GTK_LABEL (label), 25);
+  gtk_label_set_max_width_chars (GTK_LABEL (label), 25);
+  gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.5);
+  gtk_widget_set_halign (label, GTK_ALIGN_START);
+  gtk_label_set_ellipsize (GTK_LABEL (label), PANGO_ELLIPSIZE_MIDDLE);
+
+  return label;
+}
+
 static void
 ido_media_player_menu_item_init (IdoMediaPlayerMenuItem *self)
 {
@@ -140,19 +146,12 @@
   gtk_widget_set_size_request (self->album_art, ALBUM_ART_SIZE, ALBUM_ART_SIZE);
   gtk_widget_set_margin_right (self->album_art, 8);
 
-  self->artist_label = gtk_label_new (NULL);
-  gtk_widget_set_halign (self->artist_label, GTK_ALIGN_START);
-  gtk_label_set_ellipsize (GTK_LABEL (self->artist_label), PANGO_ELLIPSIZE_MIDDLE);
-
-  self->piece_label = gtk_label_new (NULL);
-  gtk_widget_set_halign (self->piece_label, GTK_ALIGN_START);
-  gtk_label_set_ellipsize (GTK_LABEL (self->piece_label), PANGO_ELLIPSIZE_MIDDLE);
-
-  self->container_label = gtk_label_new (NULL);
-  gtk_widget_set_halign (self->container_label, GTK_ALIGN_START);
+  self->artist_label = track_info_label_new ();
+  self->piece_label = track_info_label_new ();
+
+  self->container_label = track_info_label_new ();
+  gtk_widget_set_vexpand (self->container_label, TRUE);
   gtk_widget_set_valign (self->container_label, GTK_ALIGN_START);
-  gtk_widget_set_vexpand (self->container_label, TRUE);
-  gtk_label_set_ellipsize (GTK_LABEL (self->container_label), PANGO_ELLIPSIZE_MIDDLE);
 
   self->metadata_widget = gtk_grid_new ();
   gtk_grid_attach (GTK_GRID (self->metadata_widget), self->album_art, 0, 0, 1, 4);
@@ -274,6 +273,22 @@
 }
 
 static void
+gtk_label_set_markup_printf_escaped (GtkLabel    *label,
+                                     const gchar *format,
+                                     ...)
+{
+  va_list args;
+  gchar *str;
+
+  va_start (args, format);
+  str = g_markup_vprintf_escaped (format, args);
+  gtk_label_set_markup (label, str);
+  va_end (args);
+
+  g_free (str);
+}
+
+static void
 ido_media_player_menu_item_set_metadata (IdoMediaPlayerMenuItem *self,
                                          const gchar            *title,
                                          const gchar            *artist,
@@ -293,9 +308,9 @@
     }
   else
     {
-      gtk_label_set_label (GTK_LABEL (self->piece_label), title);
-      gtk_label_set_label (GTK_LABEL (self->artist_label), artist);
-      gtk_label_set_label (GTK_LABEL (self->container_label), album);
+      gtk_label_set_markup_printf_escaped (GTK_LABEL (self->piece_label), "<small>%s</small>", title);
+      gtk_label_set_markup_printf_escaped (GTK_LABEL (self->artist_label), "<small>%s</small>", artist);
+      gtk_label_set_markup_printf_escaped (GTK_LABEL (self->container_label), "<small>%s</small>", album);
       ido_media_player_menu_item_set_album_art (self, art_url);
       gtk_widget_show (self->metadata_widget);
     }

=== modified file 'src/idoplaybackmenuitem.c'
--- src/idoplaybackmenuitem.c	2014-03-28 11:27:41 +0000
+++ src/idoplaybackmenuitem.c	2014-04-23 14:37:56 +0000
@@ -27,6 +27,48 @@
 #include <gdk/gdkkeysyms.h>
 #include <math.h>
 
+#define RECT_WIDTH 130.0f
+#define Y 7.0f
+#define INNER_RADIUS 12.5
+#define MIDDLE_RADIUS 13.0f
+#define OUTER_RADIUS  14.5f
+#define CIRCLE_RADIUS 21.0f
+#define PREV_WIDTH  25.0f
+#define PREV_HEIGHT 17.0f
+#define NEXT_WIDTH  25.0f //PREV_WIDTH
+#define NEXT_HEIGHT 17.0f //PREV_HEIGHT
+#define TRI_WIDTH  11.0f
+#define TRI_HEIGHT 13.0f
+#define TRI_OFFSET  6.0f
+#define PREV_X -2.0f
+#define PREV_Y 13.0f
+#define NEXT_X 76.0f //prev_y
+#define NEXT_Y 13.0f //prev_y
+#define PAUSE_WIDTH 21.0f
+#define PAUSE_HEIGHT 27.0f
+#define BAR_WIDTH 4.5f
+#define BAR_HEIGHT 24.0f
+#define BAR_OFFSET 10.0f
+#define PAUSE_X 41.0f
+#define PAUSE_Y 7.0f
+#define PLAY_WIDTH 28.0f
+#define PLAY_HEIGHT 29.0f
+#define PLAY_PADDING 5.0f
+#define INNER_START_SHADE 0.98
+#define INNER_END_SHADE 0.98
+#define MIDDLE_START_SHADE 1.0
+#define MIDDLE_END_SHADE 1.0
+#define OUTER_START_SHADE 0.75
+#define OUTER_END_SHADE 1.3
+#define SHADOW_BUTTON_SHADE 0.8
+#define OUTER_PLAY_START_SHADE 0.7
+#define OUTER_PLAY_END_SHADE 1.38
+#define BUTTON_START_SHADE 1.1
+#define BUTTON_END_SHADE 0.9
+#define BUTTON_SHADOW_SHADE 0.8
+#define INNER_COMPRESSED_START_SHADE 1.0
+#define INNER_COMPRESSED_END_SHADE 1.0
+
 typedef enum
 {
   STATE_PAUSED,
@@ -90,10 +132,14 @@
 }
 
 static Button
-ido_playback_menu_item_get_button_at_pos (gint x,
-                                          gint y)
+ido_playback_menu_item_get_button_at_pos (GtkWidget *item,
+                                          gint       x,
+                                          gint       y)
 {
-  /*     57   101     143   187
+  GtkAllocation alloc;
+  gint left;
+
+  /*     0    44      86    130
    *  5        +------+
    * 12  +-----+      +-----+
    *     |prev   play   next|
@@ -101,13 +147,16 @@
    * 47        +------+
    */
 
-  if (x > 57 && x < 102 && y > 12 && y < 40)
+  gtk_widget_get_allocation (item, &alloc);
+  left = alloc.x + (alloc.width - RECT_WIDTH) / 2;
+
+  if (x > left && x < left + 44 && y > 12 && y < 40)
     return BUTTON_PREVIOUS;
 
-  if (x > 101 && x < 143 && y > 5 && y < 47)
+  if (x > left + 44 && x < left + 86 && y > 5 && y < 47)
     return BUTTON_PLAYPAUSE;
 
-  if (x > 142 && x < 187 && y > 12 && y < 40)
+  if (x > left + 86 && x < left + 130 && y > 12 && y < 40)
     return BUTTON_NEXT;
 
   return BUTTON_NONE;
@@ -232,7 +281,7 @@
 {
   IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (menuitem);
 
-  item->cur_pushed_button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
+  item->cur_pushed_button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
   gtk_widget_queue_draw (menuitem);
 
   return TRUE;
@@ -246,7 +295,7 @@
   Button button;
   const gchar *action = action;
 
-  button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
+  button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
   if (button != item->cur_pushed_button)
     button = BUTTON_NONE;
 
@@ -266,7 +315,7 @@
 {
   IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (menuitem);
 
-  item->cur_hover_button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
+  item->cur_hover_button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
   gtk_widget_queue_draw (menuitem);
 
   return TRUE;
@@ -426,49 +475,6 @@
  * Drawing
  */
 
-#define RECT_WIDTH 130.0f
-#define Y 7.0f
-#define X 70.0f
-#define INNER_RADIUS 12.5
-#define MIDDLE_RADIUS 13.0f
-#define OUTER_RADIUS  14.5f
-#define CIRCLE_RADIUS 21.0f
-#define PREV_WIDTH  25.0f
-#define PREV_HEIGHT 17.0f
-#define NEXT_WIDTH  25.0f //PREV_WIDTH
-#define NEXT_HEIGHT 17.0f //PREV_HEIGHT
-#define TRI_WIDTH  11.0f
-#define TRI_HEIGHT 13.0f
-#define TRI_OFFSET  6.0f
-#define PREV_X 68.0f
-#define PREV_Y 13.0f
-#define NEXT_X 146.0f
-#define NEXT_Y 13.0f //prev_y
-#define PAUSE_WIDTH 21.0f
-#define PAUSE_HEIGHT 27.0f
-#define BAR_WIDTH 4.5f
-#define BAR_HEIGHT 24.0f
-#define BAR_OFFSET 10.0f
-#define PAUSE_X 111.0f
-#define PAUSE_Y 7.0f
-#define PLAY_WIDTH 28.0f
-#define PLAY_HEIGHT 29.0f
-#define PLAY_PADDING 5.0f
-#define INNER_START_SHADE 0.98
-#define INNER_END_SHADE 0.98
-#define MIDDLE_START_SHADE 1.0
-#define MIDDLE_END_SHADE 1.0
-#define OUTER_START_SHADE 0.75
-#define OUTER_END_SHADE 1.3
-#define SHADOW_BUTTON_SHADE 0.8
-#define OUTER_PLAY_START_SHADE 0.7
-#define OUTER_PLAY_END_SHADE 1.38
-#define BUTTON_START_SHADE 1.1
-#define BUTTON_END_SHADE 0.9
-#define BUTTON_SHADOW_SHADE 0.8
-#define INNER_COMPRESSED_START_SHADE 1.0
-#define INNER_COMPRESSED_END_SHADE 1.0
-
 typedef struct
 {
   double r;
@@ -1145,6 +1151,11 @@
 ido_playback_menu_item_draw (GtkWidget* button, cairo_t *cr)
 {
   IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (button);
+  GtkAllocation alloc;
+  gint X;
+  gint abs_pause_x;
+  gint abs_prev_x;
+  gint abs_next_x;
 
   g_return_val_if_fail(IDO_IS_PLAYBACK_MENU_ITEM (button), FALSE);
   g_return_val_if_fail(cr != NULL, FALSE);
@@ -1228,6 +1239,11 @@
   double INNER_COMPRESSED_END[] = {color_inner_compressed[1].r, color_inner_compressed[1].g, color_inner_compressed[1].b, 1.0f};
   double INNER_COMPRESSED_START[] = {color_inner_compressed[0].r, color_inner_compressed[0].g, color_inner_compressed[0].b, 1.0f};
 
+  gtk_widget_get_allocation (button, &alloc);
+  X = alloc.x + (alloc.width - RECT_WIDTH) / 2 + OUTER_RADIUS;
+  abs_pause_x = X + PAUSE_X;
+  abs_prev_x = X + PREV_X;
+  abs_next_x = X + NEXT_X;
 
   draw_gradient (cr,
                  X,
@@ -1447,7 +1463,7 @@
            BUTTON_SHADOW_FOCUS,
            FALSE);
     _surface_blur (surf, 3);
-    _finalize_repaint (cr, &cr_surf, &surf, PREV_X, PREV_Y + 0.5f, 3);
+    _finalize_repaint (cr, &cr_surf, &surf, abs_prev_x, PREV_Y + 0.5f, 3);
   }
   else
   {
@@ -1467,7 +1483,7 @@
            BUTTON_SHADOW,
            FALSE);
     _surface_blur (surf, 1);
-    _finalize (cr, &cr_surf, &surf, PREV_X, PREV_Y + 1.0f);
+    _finalize (cr, &cr_surf, &surf, abs_prev_x, PREV_Y + 1.0f);
   }
 
   // draw previous-button
@@ -1486,7 +1502,7 @@
        BUTTON_START,
        BUTTON_END,
        FALSE);
-  _finalize (cr, &cr_surf, &surf, PREV_X, PREV_Y);
+  _finalize (cr, &cr_surf, &surf, abs_prev_x, PREV_Y);
 
   // draw next-button drop-shadow
   if ((item->cur_pushed_button == BUTTON_NEXT && item->keyboard_activated) ||
@@ -1508,7 +1524,7 @@
            BUTTON_SHADOW_FOCUS,
            FALSE);
     _surface_blur (surf, 3);
-    _finalize_repaint (cr, &cr_surf, &surf, NEXT_X, NEXT_Y + 0.5f, 3);
+    _finalize_repaint (cr, &cr_surf, &surf, abs_next_x, NEXT_Y + 0.5f, 3);
   }
   else
   {
@@ -1528,7 +1544,7 @@
            BUTTON_SHADOW,
            FALSE);
     _surface_blur (surf, 1);
-    _finalize (cr, &cr_surf, &surf, NEXT_X, NEXT_Y + 1.0f);
+    _finalize (cr, &cr_surf, &surf, abs_next_x, NEXT_Y + 1.0f);
   }
 
   // draw next-button
@@ -1547,7 +1563,7 @@
          BUTTON_START,
          BUTTON_END,
          FALSE);
-  _finalize (cr, &cr_surf, &surf, NEXT_X, NEXT_Y);
+  _finalize (cr, &cr_surf, &surf, abs_next_x, NEXT_Y);
 
   // draw pause-button drop-shadow
   if (item->current_state == STATE_PLAYING)
@@ -1572,7 +1588,7 @@
              BUTTON_SHADOW_FOCUS,
              TRUE);
       _surface_blur (surf, 3);
-      _finalize_repaint (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y + 0.5f, 3);
+      _finalize_repaint (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y + 0.5f, 3);
     }
     else
     {
@@ -1592,7 +1608,7 @@
              BUTTON_SHADOW,
              TRUE);
       _surface_blur (surf, 1);
-      _finalize (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y + 1.0f);
+      _finalize (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y + 1.0f);
     }
 
     // draw pause-button
@@ -1611,7 +1627,7 @@
            BUTTON_START,
            BUTTON_END,
            TRUE);
-    _finalize (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y);
+    _finalize (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y);
   }
   else if (item->current_state == STATE_PAUSED)
   {
@@ -1634,7 +1650,7 @@
              BUTTON_SHADOW_FOCUS,
              FALSE);
       _surface_blur (surf, 3);
-      _finalize_repaint (cr, &cr_surf, &surf, PAUSE_X-0.5f, PAUSE_Y + 0.5f, 3);
+      _finalize_repaint (cr, &cr_surf, &surf, abs_pause_x-0.5f, PAUSE_Y + 0.5f, 3);
     }
     else
     {
@@ -1653,7 +1669,7 @@
              BUTTON_SHADOW,
              FALSE);
       _surface_blur (surf, 1);
-      _finalize (cr, &cr_surf, &surf, PAUSE_X-0.75f, PAUSE_Y + 1.0f);
+      _finalize (cr, &cr_surf, &surf, abs_pause_x-0.75f, PAUSE_Y + 1.0f);
     }
 
     // draw play-button
@@ -1674,7 +1690,7 @@
            BUTTON_START,
            BUTTON_END,
            FALSE);
-    _finalize (cr, &cr_surf, &surf, PAUSE_X-0.5f, PAUSE_Y);
+    _finalize (cr, &cr_surf, &surf, abs_pause_x-0.5f, PAUSE_Y);
   }
   else if (item->current_state == STATE_LAUNCHING)
   {