Bug 331272 - track change notification should have Next button
track change notification should have Next button
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: User Interface
0.9.x
Other Linux
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
:
: 336088 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2006-02-15 13:00 UTC by lexual
Modified: 2009-06-01 02:29 UTC (History)
6 users (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (4.57 KB, patch)
2006-11-25 22:11 UTC, Steve Frécinaux
none Details | Diff | Review
how it looks like with the patch applied (15.57 KB, image/png)
2006-11-27 21:19 UTC, Steve Frécinaux
  Details
A different approach (7.14 KB, patch)
2008-07-02 13:01 UTC, Szilveszter Ördög
none Details | Diff | Review
The look of the notification with the patch applied (17.79 KB, image/png)
2008-07-02 13:06 UTC, Szilveszter Ördög
  Details
Updated patch to the latest version (12.75 KB, patch)
2009-05-28 08:41 UTC, Szilveszter Ördög
needs-work Details | Diff | Review
Patch v3 (2.99 KB, patch)
2009-05-30 01:35 UTC, Szilveszter Ördög
committed Details | Diff | Review

Description lexual 2006-02-15 13:00:33 UTC
The track change notification popup should have a next button.
If you want to check out a working version of this see kde's juk program. Although I'm not a big fan of it's fade in and out property, you can see what this feature would be like.

Here's a use case example.

Rhythmbox is open and playing but you are working in another window [browser,nautilus,whatever].

Notification popups with the next track.

User thinks one of:
a) That song sucks, I want to skip it.
b) I'm not in the mood for that song right now, I want to skip it.
c) I only listened to that song recently, I want to skip it.
d) This song has some strong language which would be inappropriate to be played right now, I want to skip it.

User hits the notification popup's "next" button once or multiple times to skip to a track that they would like to listen at this time. They can do this without having to leave the current window they are working in.

Additionally, a "previous" button could be added. I'm less sure about whether it is needed. The main case I see for it would be:

User is working in a window other than rhythmbox.

Song they love has just finished and notification popups announcing the next track.

User hits "previous" button to listen to the song that they loved again.
Comment 1 Sergej Kotliar 2006-02-16 01:08:16 UTC
IIRC the notification windows are intended for notification only and nothing else...
And you have the option of Right-clicking the notification icon and pressing Next/Previous. 
Admittedly - it's not as easy as what you proposed, but it's not that far away. 
I'm not sure buttons would be possible because of libnotify being desktop-wide, and only for notification, and then consider that notifications disappear after a while - what happens if a user closes his firefox window by accident because he misses the disappeared notification?
And also - it's possible to set a keyboard key to do that, which would be even easier, and of course remote controls and multimedia keyboards...
Comment 2 Sergej Kotliar 2006-03-26 23:14:41 UTC
*** Bug 336088 has been marked as a duplicate of this bug. ***
Comment 3 Steve Frécinaux 2006-11-25 17:18:24 UTC
Note that a few apps have an icon on the notification area, tbw IM apps.

Right-clicking the notification area and hitting the next button takes *much* more time than clicking on a "skip" button that would already be on screen (because it takes time to show the popup and find the menu entry, especially if your computer is slow/working).

More, it gets really annoying if you want to skip several songs (it can happen with the random mode if you're unlucky)
Comment 4 Steve Frécinaux 2006-11-25 22:11:56 UTC
Created attachment 77142 [details] [review]
Patch

This patch adds a "skip" button on the notification bubble.

Probably not the cleanest way to do it but it works.
Comment 5 Steve Frécinaux 2006-11-27 21:19:46 UTC
Created attachment 77246 [details]
how it looks like with the patch applied
Comment 6 James "Doc" Livingston 2007-01-31 11:13:28 UTC
This sounds like a good idea, but there are a few implementation issues:

rb_tray_icon_notify() gets called for thing other than changing tracks. We shouldn't show the action when a podcast download finishes (or at least, show a "queue podcast" action).

"Skip" should have _() around it, so that it gets translated.


Apart from that, it looks good.
Comment 7 David Prieto 2008-05-21 13:52:38 UTC
Hi,

Is there any progress on this issue?
Comment 8 Steve Frécinaux 2008-05-21 13:54:44 UTC
My patch is not good because of what James said. But I have changed my laptop since and the new one has mm keys ;-)
Comment 9 Szilveszter Ördög 2008-07-02 13:01:29 UTC
Created attachment 113854 [details] [review]
A different approach

Here's another approach to solving this problem. This solution displays a "Previous" and "Next" buttons only if the notification is about the currently playing song.
Comment 10 Szilveszter Ördög 2008-07-02 13:06:18 UTC
Created attachment 113855 [details]
The look of the notification with the patch applied
Comment 11 Szilveszter Ördög 2009-05-28 08:41:03 UTC
Created attachment 135484 [details] [review]
Updated patch to the latest version

Since the status icon code got moved to a plugin, I've updated my patch. The 'Previous' and 'Next' buttons can be enabled/disabled in the status icon plugin's configuration screen. Also, no buttons will be visible (and no preferences can be set) if the notification daemon doesn't support actions (e.g. notify-osd).
Comment 12 Jonathan Matthew 2009-05-29 01:08:43 UTC
Why do you think this needs to be configurable?  That doesn't seem very useful to me.

The 'previous' button wouldn't be very useful as-is, as you'd have to hit it within 3 seconds of the track starting to actually go to the previous track.  After that it just restarts the current track.

+	actions_are_supported = FALSE;
+	for (capability = capabilities; capability != NULL;
+	     capability = capability->next) {
+		if (strcmp ((char *)capability->data, "actions") == 0) {
+			actions_are_supported = TRUE;
+			break;
+		}
+	}
+
+	g_list_foreach (capabilities, (GFunc)g_free, NULL);
+	g_list_free (capabilities);

much easier to say:
  actions_are_supported = (g_list_find_custom (capabilities, "actions", strcmp);
  rb_list_deep_free (capabilities);

or something like that.

Comment 13 Bastien Nocera 2009-05-29 01:14:25 UTC
s/strcmp/g_strcmp0/
Comment 14 Szilveszter Ördög 2009-05-30 01:35:03 UTC
Created attachment 135592 [details] [review]
Patch v3

This incorporates the reviewers requests: the visibility of the buttons is no longer configurable, the 'Previous' button is removed and the detection of notify actions is simplified.
Comment 15 Szilveszter Ördög 2009-05-30 01:38:55 UTC
(In reply to comment #12)
> The 'previous' button wouldn't be very useful as-is, as you'd have to hit it
> within 3 seconds of the track starting to actually go to the previous track. 
> After that it just restarts the current track.

Yes, I know that. I have removed the button from the patch for now. However, we can add a parameter to the rb_shell_player_do_previous() function to specify whether we want to unconditionally skip to the previous track or not.
Comment 16 Jonathan Matthew 2009-06-01 02:29:09 UTC
I cleaned this up a bit more and committed it.  Thanks for picking this up again.

Note You need to log in before you can comment on or make changes to this bug.