Remove entirely the PassiveNotification and use the already
present Kirigami.InlineMessage to consistently display
stuff related to playlists
Details
load e.g. any file that is not a playlist
using the load playlist button, new notification is displayed
Diff Detail
- Repository
- R255 Elisa
- Branch
- playlist_notification
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 13930 Build 13948: arc lint + arc unit
+1, though these messages are currently not very useful because they're not actionable. If loading or saving of the playlist failed, what can I do about it? it's not clear. Especially for persistent messages, it's worth it to try to make them actionable somehow. Options include a better error message, a "Try again" button to preserve the illusion of user agency, etc.
src/qml/MediaPlayListView.qml | ||
---|---|---|
64 | This should be colored as a warning or error. |
I do not think it is such a good idea.
We should better try to use the passive notification by using more components from Kirigami instead of removing them.
Elisa will need to show notification to the user (and much better than now).
InlineMessage is also from Kirigami. :)
The PassiveNotification component is unfortunately very easy to abuse because its intended use case is very narrow: it's only for non-critical informational messages. The only time it's acceptable to use them for errors is when the error automatically retries itself, e.g. "network not available, retrying in 5 seconds". For any error message where the user may have to do something to recover, it's not appropriate to use a PassiveNotification. We're trying to remove these from Discover, in fact: https://bugs.kde.org/show_bug.cgi?id=403791
There are certainly more improvements possible, but step by step :)
Note that Elisa currently has 4(!) different notifications. There is
- track import notification
- top notification
- passive notification
- Kirigami.InlineMessage
The last two are only used for the media play list. The InlineMessage was already there for the "Undo" action, while the PassiveNotification was used for loading and saving. The PassiveNotification is IMHO not very visible, since it is very greyish and located over the content view.
As all buttons triggering the notification are the ones in the media play list section, I find the new location proposed in this patch quite suitable and IMHO provides a consistent and concise way of presenting the notifications
Yes, as said, I miss enough time until at least Sunday and will anyway have reduced bandwidth for two weeks.
I am perfectly okay with the main idea. I agree with the difference between PassiveNotification and InlineMessage .
Please see my comment and other comments
src/qml/MediaPlayListView.qml | ||
---|---|---|
147 | In the message and in the comment you are once talking about save and once about load. |
I tried to implement the actions for load and save playlist. However, InlineMessage seems only to be compatible with Kirigami Actions and not with the qqc2 ones, which is unfortunate
Will do, but I came to the conclusion that we need additional actions for better text, so I added the Kirigami.Action