Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
Hi Matěj, On Thu, Jun 27, 2013 at 11:52 AM, Matěj Laitl wrote: > On 27. 6. 2013 Myriam Schweingruber wrote: >> On Wed, Jun 26, 2013 at 9:34 PM, Matěj Laitl wrote: >> > Options for the "activate" (double or single click, per configuration, >> > if we so choose) action; feel free to suggest more: >> > >> > a) append track to playlist and start playing perhaps a different >> > (depends on other factors) track in the playlist, if not already >> > playing. The old behaviour. >> >> That was not the "old" behavior, the old behavior was double-click >> appends to playlist. There never was an immediate start of a track >> playing on double-click. > > In fact, there was. Steps to reproduce: > 1. git checkout de62cfb920d05f # the one before my commit > 2. build / run, ensure it is not playing on startup > 3. double-click on any artist in your collection; playback starts > > But this doesn't matter at all for the current situation, I just think this > may be the root of the misunderstanding here. Funny, it never did that for me, double-click only starts a track if you double click on it in the playlist, not in the Collection browser. So apparently what was clearly a bug never showed up on my installation :) > >> So -1 on that, as it was never working like that. > > -1 from me too, this (IMO unfortunate) behaviour was in fact the trigger for > me to have a look at it. I thought you wanted to revert to exactly this > behaviour (by saying "keep the old behaviour"), and I thought Markey insisted > on starting playback on double-click. there was indeed a big misunderstanding, we both (Mark and I) never meant that >> > b) just append to playlist. >> >> +1 > > This is fine with me and I didn't know this is fine with you. That's why I'm > suggesting there has been a terrible misunderstanding. I have been saying that all along, I never wanted a start on double-click, just append, and it did work like that for me since I can think of. Good we got this one cleared :) Regards, Myriam -- Proud member of the Amarok and KDE Community Protect your freedom and join the Fellowship of FSFE: http://www.fsfe.org Please don't send me proprietary file formats, use ISO standard ODF instead (ISO/IEC 26300) ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On 27. 6. 2013 Myriam Schweingruber wrote: > On Wed, Jun 26, 2013 at 9:34 PM, Matěj Laitl wrote: > > Options for the "activate" (double or single click, per configuration, > > if we so choose) action; feel free to suggest more: > > > > a) append track to playlist and start playing perhaps a different > > (depends on other factors) track in the playlist, if not already > > playing. The old behaviour. > > That was not the "old" behavior, the old behavior was double-click > appends to playlist. There never was an immediate start of a track > playing on double-click. In fact, there was. Steps to reproduce: 1. git checkout de62cfb920d05f # the one before my commit 2. build / run, ensure it is not playing on startup 3. double-click on any artist in your collection; playback starts But this doesn't matter at all for the current situation, I just think this may be the root of the misunderstanding here. > So -1 on that, as it was never working like that. -1 from me too, this (IMO unfortunate) behaviour was in fact the trigger for me to have a look at it. I thought you wanted to revert to exactly this behaviour (by saying "keep the old behaviour"), and I thought Markey insisted on starting playback on double-click. > > b) just append to playlist. > > +1 This is fine with me and I didn't know this is fine with you. That's why I'm suggesting there has been a terrible misunderstanding. > > c) insert after currently playing, play the track(s) immediately. The > > new > > and controversial behaviour. > > -2, it really is annoying on more than one point, not only does it add > the track, but it adds it after the current track and starts playing, > and it even adds queue markers, very disturbing. This makes an easy > append to playlist action impossible, I have been swearing a hundred > times abut this behavior since the change. Understandable that you don't want to change your habits. I'm still +1 on this but I'm fine to be outweighed on this matter. > Leave the double-click as it was, at least it doesn't disturb the way > hundreds of users are used to. ^^^ This was the source of my prolonged confusion. Regards, Matěj ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On Wed, Jun 26, 2013 at 9:34 PM, Matěj Laitl wrote: > On 16. 6. 2013 Myriam Schweingruber wrote: >> > Options for the "activate" (double or single click, per configuration, if >> > we so choose) action; feel free to suggest more: >> > >> > a) append track to playlist and start playing perhaps a different (depends >> > on other factors) track in the playlist, if not already playing. The old >> > behaviour. That was not the "old" behavior, the old behavior was double-click appends to playlist. There never was an immediate start of a track playing on double-click. So -1 on that, as it was never working like that. >> > b) just append to playlist. +1 >> > c) insert after currently playing, play the track(s) immediately. The new >> > and controversial behaviour. -2, it really is annoying on more than one point, not only does it add the track, but it adds it after the current track and starts playing, and it even adds queue markers, very disturbing. This makes an easy append to playlist action impossible, I have been swearing a hundred times abut this behavior since the change. >> > My personal preference: >> > +1 for c). rationale: double-click then behaves the same in playlist and >> > in >> > collection, some may fine it more consistent with other KDE apps like >> > Dolphin. At the same time I ack this may be too disruptive change and the >> > queue usage is weird. >> > >> > +0,75 for b). >> > >> > -1 for a). Rationale: the "start playing something arbitrary when not >> > already playing" aspect is something that really bugs me. We've had at >> > least 3 bug reports (mentioned in commit above) thinking this was a bug, >> > not something intentional, which means I'm not alone. I think that an >> > action shouldn't depend on current state (whether playing, queue state, >> > playback mode) that much. >> > >> > >> > Perhaps we could ditch all the special double-click handling (i.e. revert >> > to "activate" in Qt's sense, which is single or double click depending on >> > user configuration), ditch all the special "click even on text, not just >> > arrow, also expands" handling and go for b) then? If we agree on this, >> > I'm volunteering to implement this (requires more than just changes to >> > the enum above), unfortunately not during the next week [wait for my next >> > mail]. >> >> How about not modifying the double-click, which people are used to to >> add tracks at the bottom without starting to play, and use the middle >> click for the new feature? > > So what is your preference what should happen on double-click? a), b) or c), > or do you suggest some d)? How/if ever should we take KDE's settings (single > vs. double click) into account? Leave the double-click as it was, at least it doesn't disturb the way hundreds of users are used to. A change like you did is not a small change, but a complete change of the way Amarok works. Such a drastic chance is certain to bring us a shitstorm from the users. Since the modification of the layout in Amarok 2.0 hundreds of users have been getting used to the middle-click to append to the playlist, changing this now will certainly not go without heavy criticism. If you absolutely want a "queue and play" action, use a new one, and AFAIK the middle click was not sued before, so use that, but don't change the established actions. And just for the record and to show you how disruptive your change is: Double click in the collection (browser since 2.x) appends to playlist since version 1.0 beta, read the ChangeLog. Regards, Myriam -- Proud member of the Amarok and KDE Community Protect your freedom and join the Fellowship of FSFE: http://www.fsfe.org Please don't send me proprietary file formats, use ISO standard ODF instead (ISO/IEC 26300) ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
heya, Matěj Laitl wrote: > So what is your preference what should happen on double-click? a), b) or c), > or do you suggest some d)? How/if ever should we take KDE's settings (single > vs. double click) into account? imho: there are reasonable use cases for immediate playing, there are reasonable use cases for appending after the currently playing track and there are reasonable use cases for appending at the end of the playlist. Therefore it should be a config option. Default value: I don't care. ;-) ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On 16. 6. 2013 Myriam Schweingruber wrote: > > Options for the "activate" (double or single click, per configuration, if > > we so choose) action; feel free to suggest more: > > > > a) append track to playlist and start playing perhaps a different (depends > > on other factors) track in the playlist, if not already playing. The old > > behaviour. > > > > b) just append to playlist. > > > > c) insert after currently playing, play the track(s) immediately. The new > > and controversial behaviour. > > > > > > My personal preference: > > +1 for c). rationale: double-click then behaves the same in playlist and > > in > > collection, some may fine it more consistent with other KDE apps like > > Dolphin. At the same time I ack this may be too disruptive change and the > > queue usage is weird. > > > > +0,75 for b). > > > > -1 for a). Rationale: the "start playing something arbitrary when not > > already playing" aspect is something that really bugs me. We've had at > > least 3 bug reports (mentioned in commit above) thinking this was a bug, > > not something intentional, which means I'm not alone. I think that an > > action shouldn't depend on current state (whether playing, queue state, > > playback mode) that much. > > > > > > Perhaps we could ditch all the special double-click handling (i.e. revert > > to "activate" in Qt's sense, which is single or double click depending on > > user configuration), ditch all the special "click even on text, not just > > arrow, also expands" handling and go for b) then? If we agree on this, > > I'm volunteering to implement this (requires more than just changes to > > the enum above), unfortunately not during the next week [wait for my next > > mail]. > > How about not modifying the double-click, which people are used to to > add tracks at the bottom without starting to play, and use the middle > click for the new feature? So what is your preference what should happen on double-click? a), b) or c), or do you suggest some d)? How/if ever should we take KDE's settings (single vs. double click) into account? Matěj ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On Sat, Jun 15, 2013 at 3:28 PM, Matěj Laitl wrote: > On 15. 6. 2013 Myriam Schweingruber wrote: >> On Sat, May 25, 2013 at 1:15 PM, Matěj Laitl wrote: >> > Git commit a43e7e6f5a14307f543e7807a8d2351af027635a by Matěj Laitl. >> > Committed on 23/05/2013 at 18:54. >> > Pushed by laitl into branch 'master'. >> > >> > Make playlist-related actions consistent throughout Amarok code (behaviour >> > change) >> >> Sorry, but I can't agree with that, this behavior change is >> counterproductive as it totally disrupts how Amarok has always worked, >> and causes some serious usability issues: >> >> * When using a double click I want a track to be added to the >> playlists, but it should not start automatically, as this causes every >> track I add to start automatically. This makes "creating" playlists a >> real problem. >> >> * The track is not added at the bottom of the playlist but after the >> last played one, which is not desired. >> >> * Middle click to add to playlist might work on some mouses, but I >> just can't use that on my mouse which has a scroll pad, and our future >> Mac users will get in even worse trouble than I am. >> >> * We can't just throw over board the workflow, and an established way >> to use Amarok since the 1.x times, this will make us loose users, >> something we really don't want to. >> >> So please, let's discuss what makes sense in that commit and what is >> "too much" and return Amarok to a normal behavior. Right now I just >> can't use Amarok anymore and asking the users to totally modify a >> behavior is a NO-GO for me. > > That's fine, let's discuss what action should a double click (or, perhaps even > a single click if user hasn't explicitly configured KDE to use double-clicks) > cause. Fortunately the patch makes it easy to change this, just edit the > OnDoubleClickOnSelectedItems value in PlaylistController.h if you want to test > different behaviour locally. > > Options for the "activate" (double or single click, per configuration, if we > so > choose) action; feel free to suggest more: > > a) append track to playlist and start playing perhaps a different (depends on > other factors) track in the playlist, if not already playing. The old > behaviour. > > b) just append to playlist. > > c) insert after currently playing, play the track(s) immediately. The new and > controversial behaviour. > > > My personal preference: > +1 for c). rationale: double-click then behaves the same in playlist and in > collection, some may fine it more consistent with other KDE apps like Dolphin. > At the same time I ack this may be too disruptive change and the queue usage > is weird. > > +0,75 for b). > > -1 for a). Rationale: the "start playing something arbitrary when not already > playing" aspect is something that really bugs me. We've had at least 3 bug > reports (mentioned in commit above) thinking this was a bug, not something > intentional, which means I'm not alone. I think that an action shouldn't > depend on current state (whether playing, queue state, playback mode) that > much. > > > Perhaps we could ditch all the special double-click handling (i.e. revert to > "activate" in Qt's sense, which is single or double click depending on user > configuration), ditch all the special "click even on text, not just arrow, > also > expands" handling and go for b) then? If we agree on this, I'm volunteering to > implement this (requires more than just changes to the enum above), > unfortunately not during the next week [wait for my next mail]. How about not modifying the double-click, which people are used to to add tracks at the bottom without starting to play, and use the middle click for the new feature? That I could live with, and it would not disrupt the way it has always worked. What still disturbs me is that the track is added after the currently playing one, that is really annoying. How about adding at the end of the playlist but add it to the queue? Keep in mind that we have plans on the roadmap that will ultimately totally modify the playlist, replacing it with a queue IIRC. At least that is what I understood from the discussions in Randa... Regards, Myriam -- Proud member of the Amarok and KDE Community Protect your freedom and join the Fellowship of FSFE: http://www.fsfe.org Please don't send me proprietary file formats, use ISO standard ODF instead (ISO/IEC 26300) ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On 15.06.2013 22:37, Wyatt Epp wrote: On Sat, Jun 15, 2013 at 9:28 AM, Matěj Laitl wrote: On 15. 6. 2013 Myriam Schweingruber wrote: * When using a double click I want a track to be added to the playlists, but it should not start automatically, as this causes every track I add to start automatically. This makes "creating" playlists a real problem. I disagree. The semantic meaning of double clicking is generally well-understood as activation. That double-clicking doesn't consistently start playing the selected track immediately is unexpected. I have no strong opinions on any of the other points, but I strongly agree with this one. If a music player does not play the track immediately after double-click, I am left very confused and actually wondering about what other ways are there to get the track playing. I can't think of any other player that adds a track to the playlist on double-click except for foobar2000, and it's customizable there. Konrad ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On Sat, Jun 15, 2013 at 9:28 AM, Matěj Laitl wrote: > On 15. 6. 2013 Myriam Schweingruber wrote: >> * When using a double click I want a track to be added to the >> playlists, but it should not start automatically, as this causes every >> track I add to start automatically. This makes "creating" playlists a >> real problem. >> I disagree. The semantic meaning of double clicking is generally well-understood as activation. That double-clicking doesn't consistently start playing the selected track immediately is unexpected. >> * The track is not added at the bottom of the playlist but after the >> last played one, which is not desired. >> Coupled with the above, it should make more sense. Immediate activation implies an interruption in the current queue, not a wholesale discarding of it. Placing it at the end creates a sort of "action at a distance" because it's less likely that there will be immediate feedback that you've done anything. >> * Middle click to add to playlist might work on some mouses, but I >> just can't use that on my mouse which has a scroll pad, and our future >> Mac users will get in even worse trouble than I am. >> If you want to talk about how users use it, I've always used drag-and-drop or the context menu's "append to playlist" depending on whether I want it in a specific place or at the end. Given the confounding behaviour historically, I'm a little surprised to find there are people who depend on the clicking, and consider it adequate for constructing a playlist. >> * We can't just throw over board the workflow, and an established way >> to use Amarok since the 1.x times, this will make us loose users, >> something we really don't want to. >> This is a very strange argument to see come out now, of all times... > Options for the "activate" (double or single click, per configuration, if we > so > choose) action; feel free to suggest more: > > c) insert after currently playing, play the track(s) immediately. The new and > controversial behaviour. > I think it's abundantly clear where I stand on this. In terms of least-surprise and immediacy of feedback, this is the only one that makes real sense. If I think of a fourth path, I'll be sure to mention it. Cheers, Wyatt ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
On 15. 6. 2013 Myriam Schweingruber wrote: > On Sat, May 25, 2013 at 1:15 PM, Matěj Laitl wrote: > > Git commit a43e7e6f5a14307f543e7807a8d2351af027635a by Matěj Laitl. > > Committed on 23/05/2013 at 18:54. > > Pushed by laitl into branch 'master'. > > > > Make playlist-related actions consistent throughout Amarok code (behaviour > > change) > > Sorry, but I can't agree with that, this behavior change is > counterproductive as it totally disrupts how Amarok has always worked, > and causes some serious usability issues: > > * When using a double click I want a track to be added to the > playlists, but it should not start automatically, as this causes every > track I add to start automatically. This makes "creating" playlists a > real problem. > > * The track is not added at the bottom of the playlist but after the > last played one, which is not desired. > > * Middle click to add to playlist might work on some mouses, but I > just can't use that on my mouse which has a scroll pad, and our future > Mac users will get in even worse trouble than I am. > > * We can't just throw over board the workflow, and an established way > to use Amarok since the 1.x times, this will make us loose users, > something we really don't want to. > > So please, let's discuss what makes sense in that commit and what is > "too much" and return Amarok to a normal behavior. Right now I just > can't use Amarok anymore and asking the users to totally modify a > behavior is a NO-GO for me. That's fine, let's discuss what action should a double click (or, perhaps even a single click if user hasn't explicitly configured KDE to use double-clicks) cause. Fortunately the patch makes it easy to change this, just edit the OnDoubleClickOnSelectedItems value in PlaylistController.h if you want to test different behaviour locally. Options for the "activate" (double or single click, per configuration, if we so choose) action; feel free to suggest more: a) append track to playlist and start playing perhaps a different (depends on other factors) track in the playlist, if not already playing. The old behaviour. b) just append to playlist. c) insert after currently playing, play the track(s) immediately. The new and controversial behaviour. My personal preference: +1 for c). rationale: double-click then behaves the same in playlist and in collection, some may fine it more consistent with other KDE apps like Dolphin. At the same time I ack this may be too disruptive change and the queue usage is weird. +0,75 for b). -1 for a). Rationale: the "start playing something arbitrary when not already playing" aspect is something that really bugs me. We've had at least 3 bug reports (mentioned in commit above) thinking this was a bug, not something intentional, which means I'm not alone. I think that an action shouldn't depend on current state (whether playing, queue state, playback mode) that much. Perhaps we could ditch all the special double-click handling (i.e. revert to "activate" in Qt's sense, which is single or double click depending on user configuration), ditch all the special "click even on text, not just arrow, also expands" handling and go for b) then? If we agree on this, I'm volunteering to implement this (requires more than just changes to the enum above), unfortunately not during the next week [wait for my next mail]. Matěj ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: [amarok] /: Make playlist-related actions consistent throughout Amarok code (behaviour change)
Hi all On Sat, May 25, 2013 at 1:15 PM, Matěj Laitl wrote: > Git commit a43e7e6f5a14307f543e7807a8d2351af027635a by Matěj Laitl. > Committed on 23/05/2013 at 18:54. > Pushed by laitl into branch 'master'. > > Make playlist-related actions consistent throughout Amarok code (behaviour > change) > > This commits boasts a couple of changes, starting with the > uncontroversial ones: > > 1. The Playlist::AddOptions enum is extended with extended with > "convenience consistency" aliases: > OnDoubleClickOnSelectedItems > OnMiddleClickOnSelectedItems > OnReturnPressedOnSelectedItems > OnPlayMediaAction > OnAppendToPlaylistAction > OnReplacePlaylistAction > OnQueueToPlaylistAction > > ...and all callers of PlaylistController::insertOptioned() are modified > to use one of these values instead of the "low-level" flags like > DirectPlay that are actually tested for in the insertOptioned() > implementation. This serves that we remain consistent across Amarok > from now on. > > 2. The actual "low-level" enum values have been changed and > insertOptioned() was updated accordingly: > a) PrependToQueue, which implies Queue, was added. > b) StartPlay (start playing unless something is already playing) >was removed. No caller uses it anymore (see below) and this was >convoluted anyway, IMO. > c) DirectPlay now implies PrependToQueue. This may seem strange, >but the rationale is following: when you directplay just one >track (which is the case 90% of the time), it is played >immediately, and this should apply even when you add more >tracks. PrependToQueue makes this possible without hacks and is >invisible in case of just one track, because it is immediately >popped from the queue. Plus it has a positive side-effect of >inserting the track at a meaningful place (affects what track is >played next). > d) LoadAndPlay and LoadAndPlayImmediatelly were removed, because >they were replaces with consistency aliases. > > 3. Thanks to 2b), 2c) and implementation changes, the actual action > performed upon a certain trigger no longer depends on any state. > The state of playlist search no longer affects whether a track will > be played in case of DirectPlay. > > 4. insertOptioned() was cleaned up and changed, for example it tries > to choose the best place to insert tracks according to > PrependToQueue or Queue. > > 5. The convenience aliases were assigned as follows: > > OnDoubleClickOnSelectedItems = OnReturnPressedOnSelectedItems = > = OnPlayMediaAction = DirectPlay. > OnMiddleClickOnSelectedItems = OnAppendToPlaylistAction = Append (0). > OnReplacePlaylistAction = Replace (no-brainer). > OnQueueToPlaylistAction = Queue (no-brainer). > > These aren't of course set in stone, they were however chosen to be > as much consistent with other KDE apps as possible. > > Especially the "DirectPlay implies PrependToQueue" change is a bit > controversial, my opinion in that matter is anything but strong and I'm > open to any discussion. But perhaps try to use it for a couple of days > to get over the barrier of change. > > CHANGES: > * Playlist-related actions were harmonized: double-clicking, pressing >enter or using any "play media" action will prepend tracks to queue >and immediately start playing; middle-clicking appends to playlist; >append or replace actions will no longer start playback. > > CCMAIL: amarok-pr...@kde.org > CCMAIL: amarok-devel@kde.org > BUG: 145468 > BUG: 145490 > BUG: 194549 > FIXED-IN: 2.8 > GUI: Behavioural change in some places, to increase consistency. Please > check that the docs don't mention the old behaviour, see CHANGES. > DIGEST: Amarok harmonizes playlist-related actions (double-clicking, > pressing Enter, middle clicking...) > > M +4-0ChangeLog > M +4-5playground/src/context/applets/coverbling/CoverBlingApplet.cpp > M +1-1playground/src/context/applets/coverbling/CoverBlingApplet.h > M +1-1playground/src/context/applets/covergrid/AlbumItem.cpp > M +5-8src/App.cpp > M +5-5src/MainWindow.cpp > M +1-1src/amarokurls/BookmarkTreeView.cpp > M +8-11 src/browsers/CollectionTreeView.cpp > M +1-1src/browsers/CollectionTreeView.h > M +7-7src/browsers/filebrowser/FileView.cpp > M +1-1src/browsers/playlistbrowser/DynamicView.cpp > M +10 -12 src/browsers/playlistbrowser/PlaylistBrowserView.cpp > M +3-3src/browsers/playlistbrowser/PlaylistBrowserView.h > M +12 -6src/context/applets/albums/AlbumsView.cpp > M +2-1src/context/applets/albums/AlbumsView.h > M +2-2src/context/applets/similarartists/ArtistWidget.cpp > M +2-1src/dbus/mpris1/TrackListHandler.cpp > M +1-1src/dbus/mpris2/MediaPlayer2Player.cpp > M +63 -48 src/playlist/PlaylistController.cpp > M +19 -7src/play
