This patch moves the buttons in the playlist to the top and groups them. I also changed the clear playlist icon to the trash icon (I think it conveys the meaning better), and also tweaked the buttons text for more consistency.
Details
- Reviewers
mgallien astippich - Group Reviewers
Elisa - Commits
- R255:f3f15b0f62a1: New header style for playlist
Diff Detail
- Repository
- R255 Elisa
- Branch
- playlist_header
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 816 Build 829: arc lint + arc unit
My objection to using the term "Playlist" for this widget is that it conflicts with the same use of that term to mean "a saved list of songs you can access and play later" in most other contexts. Since the "Now Playing" string is also problematic because it conflicts with the use of that same term in the list on the left, how about instead "Playback Queue"? For the other strings, my recommendation would be:
- "Clear all"
- "Save as playlist file..."
- "Load playlist file..."
The last two need an ellipsis since they open save or open dialogs.
The trash icon could frighten people into thinking that their music will be deleted. I recommend instead using the new "broom-style" edit-clear-all icon that we just added to Breeze for just this kind of application : https://cgit.kde.org/breeze-icons.git/commit/?id=a3588fca59b2b71940272983e295cf339be68dd8
(Assuming you're okay with Frameworks 5.48 as a dependency, of course)
+1 for removing the gray bar at the bottom.
Good point in the ellipsis. The broom icon is way better but I'm going to wait for confirmation from @mgallien or @astippich since we are on 5.45 now.
Not sure about the "Playback Queue" idea though. I've always seen this element called "a playlist" (ever since the winamp days). Also, it's saved when you quit and you can save/load to a file too, so it fits the description :)
Thanks for your work.
I have two remarks.
In reversed mode, the menu is shown at the wrong place. It is shown at the other side of the playlist component.
I am not so sure about the "clear playlist" needing now two clicks. This is an action that may need to be still reachable with only one click. What do you think ?
I like the idea of removing the bar at the bottom, but imho the actions should either be all accessible through the menu or still have dedicated buttons, for consistency. Isn't there enough space to add 4 actions instead?
Yeah, it looks like there's room for some buttons up on top without needing a drop-down menu.
Hey guys, yeah there's room for the 4 buttons up there. The main pro for the dropdown was having text for the buttons that modified the playlist, but having the buttons could also simplify things. Sorry I keep forgetting about reversed mode
How about putting the icons on top and making them toolbuttons without a visible button-looking border? There's plenty of unused vertical space there, and square buttons might fit well in the available space.
Do you mean in the Playlist title row? They are already toolbuttons in the latest revision
I mean they could be buttons with:
- no visible borders (the borders appear on hover)
- the icon on top, rather than on the side
That would make them take up more of the vertical space and less of the horizontal space.
Looks like ToolButtons ignore text. I managed to hack this by shoving a Text object inside them but it really messes up the layout and only the icon is clickable.
I think having icons only would be fine, that's what tooltips are for anyways :)
@mgallien, @astippich What do you think about the clear icon? Should we revert back to the line (current one) or bump KF to 5.48 to use the new broom icon?
imho only toolbuttons is fine.
I would suggest to keep the old icon for now, since it is also somewhat unrelated to the rest. You can change it afterwards when Elisa requires KF 5.48. Since it was just released, it is probably a little bit too early.