D10973: [media controller applet] Support MPRIS services where CanPause==false

Kai Uwe Broulik noreply at phabricator.kde.org
Tue Mar 6 08:42:05 UTC 2018


broulik added a comment.


  Other than those nitpicks would be good to go I guess

INLINE COMMENTS

> main.qml:91
>              if (root.state == "playing") {
> -                plasmoid.setAction("playPause", i18nc("Pause playback", "Pause"), "media-playback-pause")
> -                plasmoid.action("playPause").enabled = Qt.binding(function() {
> -                    return root.canPause;
> -                });
> +                if (canPause) {
> +                    plasmoid.setAction("pause", i18nc("Pause playback", "Pause"), "media-playback-pause")

Shouldn't that be a binding? If the player cannot pause, the entry would just not show up rather than become disabled, ie. keep the action as "playPause" maybe?

> main.qml:149
> +                root.state === "paused" ?  "media-playback-pause" :
> +                                           "media-playback-stop"
>          active: compactMouse.containsMouse

Not a huge fan of that stopped icon in the panel

> main.qml:203
> +    function togglePlaying() {
> +        if (root.canPause) {
> +            root.action_playPause();

Please don't mix manual toggling and playPause, just do it manually then, otherwise the code becomes a bit convoluted

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D10973

To: kossebau, broulik
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180306/0f3b1cef/attachment-0001.html>


More information about the Plasma-devel mailing list