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

Proposed by Lars Karlitski on 2014-04-23
Status: Merged
Approved by: Charles Kerr on 2014-04-23
Approved revision: 186
Merged at revision: 183
Proposed branch: lp:~larsu/ido/lp1080076
Merge into: lp:ido/14.04
Diff against target: 405 lines (+119/-88)
2 files modified
src/idomediaplayermenuitem.c (+39/-24)
src/idoplaybackmenuitem.c (+80/-64)
To merge this branch: bzr merge lp:~larsu/ido/lp1080076
Reviewer Review Type Date Requested Status
Charles Kerr (community) 2014-04-23 Approve on 2014-04-23
PS Jenkins bot (community) continuous-integration Approve on 2014-04-23
Review via email: mp+216864@code.launchpad.net

Commit message

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.

Description of the change

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.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Charles Kerr (charlesk) wrote :

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 updated on 2014-04-23
186. By Lars Karlitski on 2014-04-23

idoplaybackmenuitem: don't introduce even more magic numbers

Charles Kerr (charlesk) wrote :

Thanks :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/idomediaplayermenuitem.c'
2--- src/idomediaplayermenuitem.c 2013-06-19 21:07:59 +0000
3+++ src/idomediaplayermenuitem.c 2014-04-23 14:37:56 +0000
4@@ -102,14 +102,6 @@
5 }
6
7 static void
8-ido_media_player_menu_item_get_preferred_width (GtkWidget *widget,
9- gint *minimum,
10- gint *natural)
11-{
12- *minimum = *natural = 200;
13-}
14-
15-static void
16 ido_media_player_menu_item_class_init (IdoMediaPlayerMenuItemClass *klass)
17 {
18 GObjectClass *object_class = G_OBJECT_CLASS (klass);
19@@ -117,10 +109,24 @@
20
21 object_class->dispose = ido_media_player_menu_item_dispose;
22
23- widget_class->get_preferred_width = ido_media_player_menu_item_get_preferred_width;
24 widget_class->draw = ido_media_player_menu_item_draw;
25 }
26
27+static GtkWidget *
28+track_info_label_new ()
29+{
30+ GtkWidget *label;
31+
32+ label = gtk_label_new (NULL);
33+ gtk_label_set_width_chars (GTK_LABEL (label), 25);
34+ gtk_label_set_max_width_chars (GTK_LABEL (label), 25);
35+ gtk_misc_set_alignment (GTK_MISC (label), 0.0, 0.5);
36+ gtk_widget_set_halign (label, GTK_ALIGN_START);
37+ gtk_label_set_ellipsize (GTK_LABEL (label), PANGO_ELLIPSIZE_MIDDLE);
38+
39+ return label;
40+}
41+
42 static void
43 ido_media_player_menu_item_init (IdoMediaPlayerMenuItem *self)
44 {
45@@ -140,19 +146,12 @@
46 gtk_widget_set_size_request (self->album_art, ALBUM_ART_SIZE, ALBUM_ART_SIZE);
47 gtk_widget_set_margin_right (self->album_art, 8);
48
49- self->artist_label = gtk_label_new (NULL);
50- gtk_widget_set_halign (self->artist_label, GTK_ALIGN_START);
51- gtk_label_set_ellipsize (GTK_LABEL (self->artist_label), PANGO_ELLIPSIZE_MIDDLE);
52-
53- self->piece_label = gtk_label_new (NULL);
54- gtk_widget_set_halign (self->piece_label, GTK_ALIGN_START);
55- gtk_label_set_ellipsize (GTK_LABEL (self->piece_label), PANGO_ELLIPSIZE_MIDDLE);
56-
57- self->container_label = gtk_label_new (NULL);
58- gtk_widget_set_halign (self->container_label, GTK_ALIGN_START);
59+ self->artist_label = track_info_label_new ();
60+ self->piece_label = track_info_label_new ();
61+
62+ self->container_label = track_info_label_new ();
63+ gtk_widget_set_vexpand (self->container_label, TRUE);
64 gtk_widget_set_valign (self->container_label, GTK_ALIGN_START);
65- gtk_widget_set_vexpand (self->container_label, TRUE);
66- gtk_label_set_ellipsize (GTK_LABEL (self->container_label), PANGO_ELLIPSIZE_MIDDLE);
67
68 self->metadata_widget = gtk_grid_new ();
69 gtk_grid_attach (GTK_GRID (self->metadata_widget), self->album_art, 0, 0, 1, 4);
70@@ -274,6 +273,22 @@
71 }
72
73 static void
74+gtk_label_set_markup_printf_escaped (GtkLabel *label,
75+ const gchar *format,
76+ ...)
77+{
78+ va_list args;
79+ gchar *str;
80+
81+ va_start (args, format);
82+ str = g_markup_vprintf_escaped (format, args);
83+ gtk_label_set_markup (label, str);
84+ va_end (args);
85+
86+ g_free (str);
87+}
88+
89+static void
90 ido_media_player_menu_item_set_metadata (IdoMediaPlayerMenuItem *self,
91 const gchar *title,
92 const gchar *artist,
93@@ -293,9 +308,9 @@
94 }
95 else
96 {
97- gtk_label_set_label (GTK_LABEL (self->piece_label), title);
98- gtk_label_set_label (GTK_LABEL (self->artist_label), artist);
99- gtk_label_set_label (GTK_LABEL (self->container_label), album);
100+ gtk_label_set_markup_printf_escaped (GTK_LABEL (self->piece_label), "<small>%s</small>", title);
101+ gtk_label_set_markup_printf_escaped (GTK_LABEL (self->artist_label), "<small>%s</small>", artist);
102+ gtk_label_set_markup_printf_escaped (GTK_LABEL (self->container_label), "<small>%s</small>", album);
103 ido_media_player_menu_item_set_album_art (self, art_url);
104 gtk_widget_show (self->metadata_widget);
105 }
106
107=== modified file 'src/idoplaybackmenuitem.c'
108--- src/idoplaybackmenuitem.c 2014-03-28 11:27:41 +0000
109+++ src/idoplaybackmenuitem.c 2014-04-23 14:37:56 +0000
110@@ -27,6 +27,48 @@
111 #include <gdk/gdkkeysyms.h>
112 #include <math.h>
113
114+#define RECT_WIDTH 130.0f
115+#define Y 7.0f
116+#define INNER_RADIUS 12.5
117+#define MIDDLE_RADIUS 13.0f
118+#define OUTER_RADIUS 14.5f
119+#define CIRCLE_RADIUS 21.0f
120+#define PREV_WIDTH 25.0f
121+#define PREV_HEIGHT 17.0f
122+#define NEXT_WIDTH 25.0f //PREV_WIDTH
123+#define NEXT_HEIGHT 17.0f //PREV_HEIGHT
124+#define TRI_WIDTH 11.0f
125+#define TRI_HEIGHT 13.0f
126+#define TRI_OFFSET 6.0f
127+#define PREV_X -2.0f
128+#define PREV_Y 13.0f
129+#define NEXT_X 76.0f //prev_y
130+#define NEXT_Y 13.0f //prev_y
131+#define PAUSE_WIDTH 21.0f
132+#define PAUSE_HEIGHT 27.0f
133+#define BAR_WIDTH 4.5f
134+#define BAR_HEIGHT 24.0f
135+#define BAR_OFFSET 10.0f
136+#define PAUSE_X 41.0f
137+#define PAUSE_Y 7.0f
138+#define PLAY_WIDTH 28.0f
139+#define PLAY_HEIGHT 29.0f
140+#define PLAY_PADDING 5.0f
141+#define INNER_START_SHADE 0.98
142+#define INNER_END_SHADE 0.98
143+#define MIDDLE_START_SHADE 1.0
144+#define MIDDLE_END_SHADE 1.0
145+#define OUTER_START_SHADE 0.75
146+#define OUTER_END_SHADE 1.3
147+#define SHADOW_BUTTON_SHADE 0.8
148+#define OUTER_PLAY_START_SHADE 0.7
149+#define OUTER_PLAY_END_SHADE 1.38
150+#define BUTTON_START_SHADE 1.1
151+#define BUTTON_END_SHADE 0.9
152+#define BUTTON_SHADOW_SHADE 0.8
153+#define INNER_COMPRESSED_START_SHADE 1.0
154+#define INNER_COMPRESSED_END_SHADE 1.0
155+
156 typedef enum
157 {
158 STATE_PAUSED,
159@@ -90,10 +132,14 @@
160 }
161
162 static Button
163-ido_playback_menu_item_get_button_at_pos (gint x,
164- gint y)
165+ido_playback_menu_item_get_button_at_pos (GtkWidget *item,
166+ gint x,
167+ gint y)
168 {
169- /* 57 101 143 187
170+ GtkAllocation alloc;
171+ gint left;
172+
173+ /* 0 44 86 130
174 * 5 +------+
175 * 12 +-----+ +-----+
176 * |prev play next|
177@@ -101,13 +147,16 @@
178 * 47 +------+
179 */
180
181- if (x > 57 && x < 102 && y > 12 && y < 40)
182+ gtk_widget_get_allocation (item, &alloc);
183+ left = alloc.x + (alloc.width - RECT_WIDTH) / 2;
184+
185+ if (x > left && x < left + 44 && y > 12 && y < 40)
186 return BUTTON_PREVIOUS;
187
188- if (x > 101 && x < 143 && y > 5 && y < 47)
189+ if (x > left + 44 && x < left + 86 && y > 5 && y < 47)
190 return BUTTON_PLAYPAUSE;
191
192- if (x > 142 && x < 187 && y > 12 && y < 40)
193+ if (x > left + 86 && x < left + 130 && y > 12 && y < 40)
194 return BUTTON_NEXT;
195
196 return BUTTON_NONE;
197@@ -232,7 +281,7 @@
198 {
199 IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (menuitem);
200
201- item->cur_pushed_button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
202+ item->cur_pushed_button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
203 gtk_widget_queue_draw (menuitem);
204
205 return TRUE;
206@@ -246,7 +295,7 @@
207 Button button;
208 const gchar *action = action;
209
210- button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
211+ button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
212 if (button != item->cur_pushed_button)
213 button = BUTTON_NONE;
214
215@@ -266,7 +315,7 @@
216 {
217 IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (menuitem);
218
219- item->cur_hover_button = ido_playback_menu_item_get_button_at_pos (event->x, event->y);
220+ item->cur_hover_button = ido_playback_menu_item_get_button_at_pos (menuitem, event->x, event->y);
221 gtk_widget_queue_draw (menuitem);
222
223 return TRUE;
224@@ -426,49 +475,6 @@
225 * Drawing
226 */
227
228-#define RECT_WIDTH 130.0f
229-#define Y 7.0f
230-#define X 70.0f
231-#define INNER_RADIUS 12.5
232-#define MIDDLE_RADIUS 13.0f
233-#define OUTER_RADIUS 14.5f
234-#define CIRCLE_RADIUS 21.0f
235-#define PREV_WIDTH 25.0f
236-#define PREV_HEIGHT 17.0f
237-#define NEXT_WIDTH 25.0f //PREV_WIDTH
238-#define NEXT_HEIGHT 17.0f //PREV_HEIGHT
239-#define TRI_WIDTH 11.0f
240-#define TRI_HEIGHT 13.0f
241-#define TRI_OFFSET 6.0f
242-#define PREV_X 68.0f
243-#define PREV_Y 13.0f
244-#define NEXT_X 146.0f
245-#define NEXT_Y 13.0f //prev_y
246-#define PAUSE_WIDTH 21.0f
247-#define PAUSE_HEIGHT 27.0f
248-#define BAR_WIDTH 4.5f
249-#define BAR_HEIGHT 24.0f
250-#define BAR_OFFSET 10.0f
251-#define PAUSE_X 111.0f
252-#define PAUSE_Y 7.0f
253-#define PLAY_WIDTH 28.0f
254-#define PLAY_HEIGHT 29.0f
255-#define PLAY_PADDING 5.0f
256-#define INNER_START_SHADE 0.98
257-#define INNER_END_SHADE 0.98
258-#define MIDDLE_START_SHADE 1.0
259-#define MIDDLE_END_SHADE 1.0
260-#define OUTER_START_SHADE 0.75
261-#define OUTER_END_SHADE 1.3
262-#define SHADOW_BUTTON_SHADE 0.8
263-#define OUTER_PLAY_START_SHADE 0.7
264-#define OUTER_PLAY_END_SHADE 1.38
265-#define BUTTON_START_SHADE 1.1
266-#define BUTTON_END_SHADE 0.9
267-#define BUTTON_SHADOW_SHADE 0.8
268-#define INNER_COMPRESSED_START_SHADE 1.0
269-#define INNER_COMPRESSED_END_SHADE 1.0
270-
271 typedef struct
272 {
273 double r;
274@@ -1145,6 +1151,11 @@
275 ido_playback_menu_item_draw (GtkWidget* button, cairo_t *cr)
276 {
277 IdoPlaybackMenuItem *item = IDO_PLAYBACK_MENU_ITEM (button);
278+ GtkAllocation alloc;
279+ gint X;
280+ gint abs_pause_x;
281+ gint abs_prev_x;
282+ gint abs_next_x;
283
284 g_return_val_if_fail(IDO_IS_PLAYBACK_MENU_ITEM (button), FALSE);
285 g_return_val_if_fail(cr != NULL, FALSE);
286@@ -1228,6 +1239,11 @@
287 double INNER_COMPRESSED_END[] = {color_inner_compressed[1].r, color_inner_compressed[1].g, color_inner_compressed[1].b, 1.0f};
288 double INNER_COMPRESSED_START[] = {color_inner_compressed[0].r, color_inner_compressed[0].g, color_inner_compressed[0].b, 1.0f};
289
290+ gtk_widget_get_allocation (button, &alloc);
291+ X = alloc.x + (alloc.width - RECT_WIDTH) / 2 + OUTER_RADIUS;
292+ abs_pause_x = X + PAUSE_X;
293+ abs_prev_x = X + PREV_X;
294+ abs_next_x = X + NEXT_X;
295
296 draw_gradient (cr,
297 X,
298@@ -1447,7 +1463,7 @@
299 BUTTON_SHADOW_FOCUS,
300 FALSE);
301 _surface_blur (surf, 3);
302- _finalize_repaint (cr, &cr_surf, &surf, PREV_X, PREV_Y + 0.5f, 3);
303+ _finalize_repaint (cr, &cr_surf, &surf, abs_prev_x, PREV_Y + 0.5f, 3);
304 }
305 else
306 {
307@@ -1467,7 +1483,7 @@
308 BUTTON_SHADOW,
309 FALSE);
310 _surface_blur (surf, 1);
311- _finalize (cr, &cr_surf, &surf, PREV_X, PREV_Y + 1.0f);
312+ _finalize (cr, &cr_surf, &surf, abs_prev_x, PREV_Y + 1.0f);
313 }
314
315 // draw previous-button
316@@ -1486,7 +1502,7 @@
317 BUTTON_START,
318 BUTTON_END,
319 FALSE);
320- _finalize (cr, &cr_surf, &surf, PREV_X, PREV_Y);
321+ _finalize (cr, &cr_surf, &surf, abs_prev_x, PREV_Y);
322
323 // draw next-button drop-shadow
324 if ((item->cur_pushed_button == BUTTON_NEXT && item->keyboard_activated) ||
325@@ -1508,7 +1524,7 @@
326 BUTTON_SHADOW_FOCUS,
327 FALSE);
328 _surface_blur (surf, 3);
329- _finalize_repaint (cr, &cr_surf, &surf, NEXT_X, NEXT_Y + 0.5f, 3);
330+ _finalize_repaint (cr, &cr_surf, &surf, abs_next_x, NEXT_Y + 0.5f, 3);
331 }
332 else
333 {
334@@ -1528,7 +1544,7 @@
335 BUTTON_SHADOW,
336 FALSE);
337 _surface_blur (surf, 1);
338- _finalize (cr, &cr_surf, &surf, NEXT_X, NEXT_Y + 1.0f);
339+ _finalize (cr, &cr_surf, &surf, abs_next_x, NEXT_Y + 1.0f);
340 }
341
342 // draw next-button
343@@ -1547,7 +1563,7 @@
344 BUTTON_START,
345 BUTTON_END,
346 FALSE);
347- _finalize (cr, &cr_surf, &surf, NEXT_X, NEXT_Y);
348+ _finalize (cr, &cr_surf, &surf, abs_next_x, NEXT_Y);
349
350 // draw pause-button drop-shadow
351 if (item->current_state == STATE_PLAYING)
352@@ -1572,7 +1588,7 @@
353 BUTTON_SHADOW_FOCUS,
354 TRUE);
355 _surface_blur (surf, 3);
356- _finalize_repaint (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y + 0.5f, 3);
357+ _finalize_repaint (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y + 0.5f, 3);
358 }
359 else
360 {
361@@ -1592,7 +1608,7 @@
362 BUTTON_SHADOW,
363 TRUE);
364 _surface_blur (surf, 1);
365- _finalize (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y + 1.0f);
366+ _finalize (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y + 1.0f);
367 }
368
369 // draw pause-button
370@@ -1611,7 +1627,7 @@
371 BUTTON_START,
372 BUTTON_END,
373 TRUE);
374- _finalize (cr, &cr_surf, &surf, PAUSE_X, PAUSE_Y);
375+ _finalize (cr, &cr_surf, &surf, abs_pause_x, PAUSE_Y);
376 }
377 else if (item->current_state == STATE_PAUSED)
378 {
379@@ -1634,7 +1650,7 @@
380 BUTTON_SHADOW_FOCUS,
381 FALSE);
382 _surface_blur (surf, 3);
383- _finalize_repaint (cr, &cr_surf, &surf, PAUSE_X-0.5f, PAUSE_Y + 0.5f, 3);
384+ _finalize_repaint (cr, &cr_surf, &surf, abs_pause_x-0.5f, PAUSE_Y + 0.5f, 3);
385 }
386 else
387 {
388@@ -1653,7 +1669,7 @@
389 BUTTON_SHADOW,
390 FALSE);
391 _surface_blur (surf, 1);
392- _finalize (cr, &cr_surf, &surf, PAUSE_X-0.75f, PAUSE_Y + 1.0f);
393+ _finalize (cr, &cr_surf, &surf, abs_pause_x-0.75f, PAUSE_Y + 1.0f);
394 }
395
396 // draw play-button
397@@ -1674,7 +1690,7 @@
398 BUTTON_START,
399 BUTTON_END,
400 FALSE);
401- _finalize (cr, &cr_surf, &surf, PAUSE_X-0.5f, PAUSE_Y);
402+ _finalize (cr, &cr_surf, &surf, abs_pause_x-0.5f, PAUSE_Y);
403 }
404 else if (item->current_state == STATE_LAUNCHING)
405 {

Subscribers

People subscribed via source and target branches