----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100630/#review1517 -----------------------------------------------------------
So I'm against merging this review request as it is right now. However, I don't think I am in any place to reject completely a feature like this since I have not been actively developing or improving the current situation when it comes to dynamic playlists. That said, I want to explain my thoughts and why I think this is a significant regression UI wise to what we currently have in Amarok. First, I want to be clear that I am not talking about the technical quality of this RR. We all know there are significant issues with the current implementation, and that the UI basically lies to the user about what is going on under the hood. Furthermore it has quite a few issues like putting many copies of the same song in the playlist if the dynamic biases only match a small subset of the users' collection. So all my comments here are purely about the UI, none about the code. I know that the code is excellent and I am sure that it fixes many bugs that were present. With the above in mind, I think the UI is significantly less easy to use, user friendly, and simply less aesthetically pleasing than the old one: 1) Overwhelming amount of unclear terminology. The Match Type combobox is a good example. In it are a whole bunch of items that a new user (including me!) will not really know how to handle. With items such as "Match meta tag", "Partition", "Unique", "And", "Or", "Album Play", "Quiz Play", "LastFM Similar", the user has lots of foreign terms to choose between with *no* way of knowing what they are. This is the only combobox in a large grey area---I don't know what the user is expected to do when seeing this. It is unclear what the workflow is meant to be---and confronting the user with a lot of hard-to-understand options makes it hard for them to navigate the UI. 2) Lack of clarity in what the Match Types do. Say the user opens a new playlist, then he selects And from the combobox (not quite knowing what it is, but trying it anyway). Then he is presented with 2 Match Type widgets. First of all, the label for Match Type here is the same label as in the "main" Match Type comobox---what's the different? Then, the user might go Or in the "main" Match Type combobox. That adds a third Match Type entry widget to the list. Now there are 3---2 of them I got by choosing And, 1 I got by choosing Or. What's the relationship between them? Which are And'ed and which are Or'ed? There's no way to tell, and I have no idea what this set of constraints is going to do. Then I just selected Not from one of the 3 match type widgets and the widget disappeared. Why did it disappear? I just chose "Album Play" at random for another one of the match types, and *all* my widgets disappeared and the "top level" combobox just changed to Album Play. That's really bizarre---I changed 1 subwidget value and all subwidgets disappeared and the top-level value changed too? Then I selected "Unique" from the top-level combobox. Nothing happened. There are no widgets. I don't know what i'm looking at or what to do right now. 3) Lack of clarity in what the UI actually means for playlist generating. I just added a match type or two and ended up with this: http://i.imgur.com/GtDk1.png . It's totally unclear to me what this means. What is being Not'ed? What is the Last.FM similar match compared to the U2 and 1987 match doing? I'm lost. Then for fun I just added a Partition entry (i'm guessing that means to partition the playlist? It doesn't say anywhere) and got this: http://i.imgur.com/ZLErF.png . Besides being overwhelmed by + and - buttons, I again simply don't know what is going on. These are just three things that popped out after trying this for a few minutes. I haven't really looked at the code or what is going on behind the scenes, but I think most users will have a similar experience. I honestly don't know how to use it and cannot easily figure it out by playing around with it---the more I try stuff, the more confused I get. I just gave my GF a 5-min test run and she immediately stopped trying stuff since she said it made her feel stupid. She had no idea what to do. I don't think this is the direction we want to bring Amarok. Exposing the internals and catering features to power users who know what they want and how to get it is explicitly not our target demographic. That's why I think this is a major major regression UI-wise and basically cripples this feature for most users. I strongly disagree with merging it as is---but I definitely would support merging in an improved UI. - Leo On Feb. 11, 2011, 5:19 p.m., Ralf Engels wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100630/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2011, 5:19 p.m.) > > > Review request for Amarok. > > > Summary > ------- > > This fix (actually not the attached patch but the dynamicplaylist branch) > changes most of the dynamic playlist. > > Main parts are the UI which was seen as confusing by many users and produced > unexpected results. > The global bias is now a "part" bias which allows you to set a part of the > collection matching a criteria. > The custom bias is now part of the bias system. I converted all existing > custom biases to the new interface just to make sure that it's usable. > The fuzzy bias was completely removed. If someone really wants to have a bias > which results in a normal distribution of one numeric value then please feel > free to add it again. For now you can define this kind of bias by defining > something like this "(rating > 1) AND (rating < 4)" > > The branch needs to be merged back to mainline and might have conflicts. > > > This addresses bugs 175163, 175172, 177627, 188360, 188360, 228738, 230773, > 232673, 233859, 260001, 260003, 265191, and, more, probably, and some. > https://bugs.kde.org/show_bug.cgi?id=175163 > https://bugs.kde.org/show_bug.cgi?id=175172 > https://bugs.kde.org/show_bug.cgi?id=177627 > https://bugs.kde.org/show_bug.cgi?id=188360 > https://bugs.kde.org/show_bug.cgi?id=188360 > https://bugs.kde.org/show_bug.cgi?id=228738 > https://bugs.kde.org/show_bug.cgi?id=230773 > https://bugs.kde.org/show_bug.cgi?id=232673 > https://bugs.kde.org/show_bug.cgi?id=233859 > https://bugs.kde.org/show_bug.cgi?id=260001 > https://bugs.kde.org/show_bug.cgi?id=260003 > https://bugs.kde.org/show_bug.cgi?id=265191 > https://bugs.kde.org/show_bug.cgi?id=and > https://bugs.kde.org/show_bug.cgi?id=more > https://bugs.kde.org/show_bug.cgi?id=probably > https://bugs.kde.org/show_bug.cgi?id=some > > > Diffs > ----- > > ChangeLog 809d5e7 > > Diff: http://git.reviewboard.kde.org/r/100630/diff > > > Testing > ------- > > Used it for a couple of times and wrote new auto tests. > Tested against bug reports. > > > Thanks, > > Ralf > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
