Bug 138125

Summary:	double-click inconsistency in sidebar
Product:	[Applications] amarok	Reporter:	Joseph K Barker <jbarker>
Component:	general	Assignee:	Amarok Developers <amarok-bugs-dist>
Status:	RESOLVED FIXED
Severity:	wishlist
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Gentoo Packages
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Joseph K Barker 2006-11-30 02:07:55 UTC

Version:            (using KDE KDE 3.5.5)
Installed from:    Gentoo Packages

There are three places in the sidebar (that I'm aware of) where double-clicking on an item will add it to the playlist. However, they all do so slightly differently. Double-clicking in the collection browser appends the item(s) to the playlist (but the current track keeps playing). Double-clicking on a radio-stream will replace the current playlist (but the current track keeps playing). Double-clicking on a podcast appends the podcast to the playlist and starts playing the podcast.

It seems to me that there should be a standardized behavior for double-clicking. My preference would be that double-clicking an item replaces the playlist with it, and then starts playing the first item (most people I've introduced to amarok (including me) seem to expect this, and I can drag'n'drop things to the playlist that I want to append them). However, I realize that there's a lot of debate on this topic, and it's more important to me that the same action have the same effect in different places.

Possible?

Comment 1 Martin Aumueller 2006-11-30 02:16:49 UTC

SVN commit 609295 by aumuell:

consistent double-click behaviour for items in sidebar
BUG: 138125


 M  +1 -0      ChangeLog  
 M  +2 -2      src/collectionbrowser.cpp  
 M  +1 -1      src/filebrowser.cpp  
 M  +1 -1      src/mediabrowser.cpp  
 M  +2 -1      src/playlist.cpp  
 M  +1 -0      src/playlist.h  
 M  +6 -10     src/playlistbrowseritem.cpp  


--- trunk/extragear/multimedia/amarok/ChangeLog #609294:609295
@@ -21,6 +21,7 @@
       you move and rename them.
 
   CHANGES:
+    * Consistent double-click behavior in sidebar. (BR 138125)
     * Propose name of currently loaded playlist when saving current one.
     * Remove support for older libmtp versions. We now require 0.0.15 or
       newer. 
--- trunk/extragear/multimedia/amarok/src/collectionbrowser.cpp #609294:609295
@@ -1264,9 +1264,9 @@
     setCurrentItem( item );
     //append and prevent doubles in playlist
     if( item->isExpandable()  ||  m_viewMode == modeIpodView )
-        Playlist::instance()->insertMedia( listSelected(), Playlist::Unique | Playlist::Append );
+        Playlist::instance()->insertMedia( listSelected(), Playlist::Unique | Playlist::Append | Playlist::StartPlay );
     else
-        Playlist::instance()->insertMedia( static_cast<CollectionItem*>( item )->url(), Playlist::Unique | Playlist::Append );
+        Playlist::instance()->insertMedia( static_cast<CollectionItem*>( item )->url(), Playlist::Unique | Playlist::Append | Playlist::StartPlay );
 
 }
 
--- trunk/extragear/multimedia/amarok/src/filebrowser.cpp #609294:609295
@@ -416,7 +416,7 @@
 inline void
 FileBrowser::activate( const KFileItem *item )
 {
-    Playlist::instance()->insertMedia( item->url(), Playlist::Unique | Playlist::Append  );
+    Playlist::instance()->insertMedia( item->url(), Playlist::Unique | Playlist::Append | Playlist::StartPlay );
 }
 
 inline void
--- trunk/extragear/multimedia/amarok/src/mediabrowser.cpp #609294:609295
@@ -1209,7 +1209,7 @@
         return;
 
     KURL::List urls = nodeBuildDragList( item );
-    Playlist::instance()->insertMedia( urls, Playlist::Unique | Playlist::Append );
+    Playlist::instance()->insertMedia( urls, Playlist::Unique | Playlist::Append | Playlist::StartPlay );
 }
 
 void
--- trunk/extragear/multimedia/amarok/src/playlist.cpp #609294:609295
@@ -479,7 +479,8 @@
         return; // don't add empty items
     }
 
-    bool directPlay = options & DirectPlay;
+    const bool isPlaying   = EngineController::engine()->state() == Engine::Playing;
+    bool directPlay = (options & DirectPlay) || ((options & StartPlay) && !isPlaying);
 
     if( options & Replace )
        clear();
--- trunk/extragear/multimedia/amarok/src/playlist.h #609294:609295
@@ -87,6 +87,7 @@
         static const int Replace    = Clear;
         static const int DirectPlay = 8;     /// start playback of the first item in the list
         static const int Unique     = 16;    /// don't insert anything already in the playlist
+        static const int StartPlay  = 32;    /// start playback of the first item in the list if nothing else playing
 
         // it's really just the *ListView parts we want to hide...
         QScrollView *qscrollview() const
--- trunk/extragear/multimedia/amarok/src/playlistbrowseritem.cpp #609294:609295
@@ -850,8 +850,7 @@
 
 void PlaylistEntry::slotDoubleClicked()
 {
-    Playlist::instance()->insertMedia( url(), Playlist::Replace );
-    Playlist::instance()->setPlaylistName( text(0), true );
+    Playlist::instance()->insertMedia( url(), Playlist::Append | Playlist::Unique | Playlist::StartPlay );
 }
 
 
@@ -1053,8 +1052,7 @@
 
 void PlaylistTrackItem::slotDoubleClicked()
 {
-    KURL::List list( url() );
-    Playlist::instance()->insertMedia( list, Playlist::DirectPlay );
+    Playlist::instance()->insertMedia( url(), Playlist::Append | Playlist::Unique | Playlist::StartPlay );
 }
 
 
@@ -1196,7 +1194,7 @@
 
 void StreamEntry::slotDoubleClicked()
 {
-    Playlist::instance()->insertMedia( url(), Playlist::DirectPlay );
+    Playlist::instance()->insertMedia( url(), Playlist::Append | Playlist::Unique | Playlist::StartPlay );
 }
 
 void StreamEntry::setup()
@@ -2144,8 +2142,7 @@
         child = child->nextSibling();
     }
 
-    Playlist::instance()->insertMedia( list, Playlist::Replace );
-    Playlist::instance()->setPlaylistName( text(0) );
+    Playlist::instance()->insertMedia( list, Playlist::Append | Playlist::Unique | Playlist::StartPlay );
     setNew( false );
 }
 
@@ -2934,7 +2931,7 @@
         list.append( localUrl() ):
         list.append( url()      );
 
-    Playlist::instance()->insertMedia( list, Playlist::DirectPlay );
+    Playlist::instance()->insertMedia( list, Playlist::Append | Playlist::Unique | Playlist::StartPlay );
     setListened();
 }
 
@@ -3476,8 +3473,7 @@
 {
     if( !query().isEmpty() )
     {
-        Playlist::instance()->insertMediaSql( query(), Playlist::Replace );
-        Playlist::instance()->setPlaylistName( text(0) );
+        Playlist::instance()->insertMediaSql( query(), Playlist::Append | Playlist::Unique | Playlist::StartPlay );
     }
 }

Comment 2 Joseph K Barker 2006-11-30 02:54:45 UTC

Gads, that was fast. Thanks!

Comment 3 shattered 2007-03-18 17:34:18 UTC

If only this was configurable...  I particularly don't like new behaviour of 'double-click on smart playlist'.