[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 126621: [Task Manager] Add support for Unity Launcher API and Application Jobs
From: "David Edmundson" <david () davidedmundson ! co ! uk>
Date: 2016-01-04 14:40:26
Message-ID: 20160104144026.6499.69183 () mimi ! kde ! org
[Download RAW message or body]
--===============6389500399700064285==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126621/#review90547
-----------------------------------------------------------
+1 from me.
Code is very neat and tidy. I don't really have any comments.
applets/taskmanager/package/contents/ui/Task.qml (line 159)
<https://git.reviewboard.kde.org/r/126621/#comment61898>
you don't have an else deleting the object.
It could be intentional, but I want to check.
applets/taskmanager/package/contents/ui/Task.qml (line 278)
<https://git.reviewboard.kde.org/r/126621/#comment61890>
Do we need a paintedWidth/paintedHeight in IconItem?
- David Edmundson
On Jan. 4, 2016, 1:50 p.m., Kai Uwe Broulik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126621/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2016, 1:50 p.m.)
>
>
> Review request for Plasma, KDE Usability, Craig Drummond, Eike Hein, and Lukáš \
> Tinkl.
>
> Bugs: 343632
> https://bugs.kde.org/show_bug.cgi?id=343632
>
>
> Repository: plasma-desktop
>
>
> Description
> -------
>
> This adds support for the Unity Launcher API [1] with which applications can show a \
> progress indicator, a number badge as well as demand the user's attention. It also \
> shows application progress (such as copying a file) with the respective \
> application.
> This functionality has been present in the Icon Tasks applet [2] in Plasma 4 times. \
> libunity must be present for most applications to actually use this.
> I did not want to pollute libtaskmanager with this, so I made it a plugin in the \
> Task Manager applet, which should also be fairly trivial to update to the new \
> Launcher API that Unity 8 is going to use [3], or if we come up with our own \
> solution at some point, maybe.
> The primary focus is still the icons only task manager as the Unity API works on a \
> per-application/per-launcher basis rather than per-window, however, where it makes \
> sense the functonality is also offered to the traditional task manager. A downside \
> of this is that (in the current implementation, anyways) in case you do not group \
> your tasks and you have multiple windows of the same task open, it will show the \
> same information on all entries.
> For progress it gradually fills up the background of the task (VDG: I need a better \
> graphic here, I currently abuse the "hover" tasks SVG, also I find the "+2" badge \
> less than optimal) in all cases. The badges are only shown if 1) the cell is large \
> enough 2) the label is *not* shown (ie. icon tasks manager or the regular one in \
> very narrow). The badge is just a circle in theme highlight color, not an actual \
> SVG - it cuts off part of the icon to provide more contrast. The "urgent" state \
> just highlights the entry as if it requested attention, there's no wiggle animation \
> and this is nowhere integrated with the window manager, also I did not find an \
> application that used this, so I have no idea how it's used by them.
> A video with the regular task manager on the top, a regular task manager and an \
> icon tasks applet in the left panel and an icon tasks applet on the desktop (the \
> latter is broken in the video, fixed in this patch) can be found here [4].
> (There's still a ton of qDebug in the code at this point)
>
> [1] https://wiki.ubuntu.com/Unity/LauncherAPI
> [2] http://kde-apps.org/content/show.php/Icon+Tasks?content=144808
> [3] http://bazaar.launchpad.net/~unity-team/unity8/trunk/files/head:/plugins/Unity/Launcher/
> [4] https://www.youtube.com/watch?v=aBPGjlP6Wd8
>
>
> Diffs
> -----
>
> applets/taskmanager/CMakeLists.txt 3c94cb7
> applets/taskmanager/package/contents/ui/Task.qml 2a88886
> applets/taskmanager/plugin/smartlaunchers/smartlauncherbackend.h PRE-CREATION
> applets/taskmanager/plugin/smartlaunchers/smartlauncherbackend.cpp PRE-CREATION
> applets/taskmanager/plugin/smartlaunchers/smartlauncheritem.h PRE-CREATION
> applets/taskmanager/plugin/smartlaunchers/smartlauncheritem.cpp PRE-CREATION
> applets/taskmanager/plugin/taskmanagerplugin.cpp 1be1fed
>
> Diff: https://git.reviewboard.kde.org/r/126621/diff/
>
>
> Testing
> -------
>
> I started a download in Chromium and got progress as well as a badge indicating how \
> many download jobs were in progress. Thunderbird also registered a launcher on DBus \
> but it never actually set a badge although I had unread mail. I copied a folder in \
> Dolphin and got proper progress; also I played around with JobViewServer on DBus \
> and it behaved correctly, even in case there were multiple progresses in one \
> application.
>
> File Attachments
> ----------------
>
> Unity Launchers
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/03/c4c552a5-0449-47b1-a7f2-35ac20886cd0__unitylauncherstuff.png
> Two unrelated windows
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/03/3817426d-42e7-4aca-9478-2774f6908589__twounrelatedlaunchers.png
> Dolphin copying a file (notification applet next to it for demo)
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/04/4f200297-2c88-45fd-a986-8285203b6950__jobviewprogress.png
>
>
> Thanks,
>
> Kai Uwe Broulik
>
>
--===============6389500399700064285==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126621/">https://git.reviewboard.kde.org/r/126621/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">+1 \
from me.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Code is very neat and tidy. I don't really have any \
comments.</p></pre> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/126621/diff/2/?file=428372#file428372line159" \
style="color: black; font-weight: bold; text-decoration: \
underline;">applets/taskmanager/package/contents/ui/Task.qml</a> <span \
style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">159</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">if</span> <span class="p">(</span><span \
class="nx">smartLauncherEnabled</span> <span class="o">&&</span> <span \
class="o">!</span><span class="nx">smartLauncherItem</span><span class="p">)</span> \
<span class="p">{</span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">you \
don't have an else deleting the object.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">It could be \
intentional, but I want to check.</p></pre> </div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/126621/diff/2/?file=428372#file428372line278" \
style="color: black; font-weight: bold; text-decoration: \
underline;">applets/taskmanager/package/contents/ui/Task.qml</a> <span \
style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">278</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="k">height:</span> <span class="nx">units</span><span \
class="p">.</span><span class="nx">roundToIconSize</span><span \
class="p">(</span><span class="nb">Math</span><span class="p">.</span><span \
class="nx">min</span><span class="p">(</span><span class="nx">parent</span><span \
class="p">.</span><span class="nx">width</span><span class="p">,</span> <span \
class="nx">parent</span><span class="p">.</span><span class="nx">height</span><span \
class="p">))</span> <span class="c1">// the size the icon actually \
covers</span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do we \
need a paintedWidth/paintedHeight in IconItem?</p></pre> </div>
</div>
<br />
<p>- David Edmundson</p>
<br />
<p>On January 4th, 2016, 1:50 p.m. UTC, Kai Uwe Broulik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma, KDE Usability, Craig Drummond, Eike Hein, and Lukáš \
Tinkl.</div> <div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated Jan. 4, 2016, 1:50 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=343632">343632</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This adds support for the Unity Launcher API [1] with \
which applications can show a progress indicator, a number badge as well as demand \
the user's attention. It also shows application progress (such as copying a file) \
with the respective application.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">This functionality has \
been present in the Icon Tasks applet [2] in Plasma 4 times. libunity must be present \
for most applications to actually use this.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I did not want to \
pollute libtaskmanager with this, so I made it a plugin in the Task Manager applet, \
which should also be fairly trivial to update to the new Launcher API that Unity 8 is \
going to use [3], or if we come up with our own solution at some point, maybe.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">The primary focus is still the icons only task manager as the Unity API \
works on a per-application/per-launcher basis rather than per-window, however, where \
it makes sense the functonality is also offered to the traditional task manager. A \
downside of this is that (in the current implementation, anyways) in case you do not \
group your tasks and you have multiple windows of the same task open, it will show \
the same information on all entries.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">For progress it \
gradually fills up the background of the task (VDG: I need a better graphic here, I \
currently abuse the "hover" tasks SVG, also I find the "+2" badge less than optimal) \
in all cases. The badges are only shown if 1) the cell is large enough 2) the label \
is <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">not</em> shown (ie. icon tasks manager or the regular \
one in very narrow). The badge is just a circle in theme highlight color, not an \
actual SVG - it cuts off part of the icon to provide more contrast. The "urgent" \
state just highlights the entry as if it requested attention, there's no wiggle \
animation and this is nowhere integrated with the window manager, also I did not find \
an application that used this, so I have no idea how it's used by them.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">A video with the regular task manager on the top, a regular task manager \
and an icon tasks applet in the left panel and an icon tasks applet on the desktop \
(the latter is broken in the video, fixed in this patch) can be found here [4].</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">(There's still a ton of qDebug in the code at this \
point)</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">[1] https://wiki.ubuntu.com/Unity/LauncherAPI [2] \
http://kde-apps.org/content/show.php/Icon+Tasks?content=144808 [3] \
http://bazaar.launchpad.net/~unity-team/unity8/trunk/files/head:/plugins/Unity/Launcher/
[4] https://www.youtube.com/watch?v=aBPGjlP6Wd8</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I started a download in Chromium and got progress as \
well as a badge indicating how many download jobs were in progress. Thunderbird also \
registered a launcher on DBus but it never actually set a badge although I had unread \
mail. I copied a folder in Dolphin and got proper progress; also I played around with \
JobViewServer on DBus and it behaved correctly, even in case there were multiple \
progresses in one application.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/taskmanager/CMakeLists.txt <span style="color: \
grey">(3c94cb7)</span></li>
<li>applets/taskmanager/package/contents/ui/Task.qml <span style="color: \
grey">(2a88886)</span></li>
<li>applets/taskmanager/plugin/smartlaunchers/smartlauncherbackend.h <span \
style="color: grey">(PRE-CREATION)</span></li>
<li>applets/taskmanager/plugin/smartlaunchers/smartlauncherbackend.cpp <span \
style="color: grey">(PRE-CREATION)</span></li>
<li>applets/taskmanager/plugin/smartlaunchers/smartlauncheritem.h <span \
style="color: grey">(PRE-CREATION)</span></li>
<li>applets/taskmanager/plugin/smartlaunchers/smartlauncheritem.cpp <span \
style="color: grey">(PRE-CREATION)</span></li>
<li>applets/taskmanager/plugin/taskmanagerplugin.cpp <span style="color: \
grey">(1be1fed)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126621/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/01/03/c4c552a5-0449-47b1-a7f2-35ac20886cd0__unitylauncherstuff.png">Unity \
Launchers</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/01/03/3817426d-42e7-4aca-9478-2774f6908589__twounrelatedlaunchers.png">Two \
unrelated windows</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/01/04/4f200297-2c88-45fd-a986-8285203b6950__jobviewprogress.png">Dolphin \
copying a file (notification applet next to it for demo)</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============6389500399700064285==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic