<table><tr><td style="">hein added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D3005" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Here's a few spontaneous thoughts ... not really meant as an in-depth review:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">This is like a step 1, right? It appears the requestAdd/Remove APIs currently add/remove to all activities, so there's no way yet for users to make per-activity launchers?</li>
</ul>
<ul class="remarkup-list">
<li class="remarkup-list-item">I'm a bit surprised that the code doesn't use the existing machinery to filter the launcher list by activity ... couldn't LauncherTasksModel just implement AbstractTasksModel::AdditionalRoles::Activities for launcher tasks and rely on TaskFilterProxyModel doing the rest, just like it works for window tasks? The "shownLauncherList" stuff seems to reinvent the wheel at the lower level, when the overall approach in the libtaskmanager codebase is to have a pyramid of dumb list source models that get increasingly filtered/mangled. Or did you try this and it ran into trouble with the ordering ...?</li>
</ul>
<ul class="remarkup-list">
<li class="remarkup-list-item">Note that there shouldn't be any magic that only makes sense in concert with the specific TM applet, the libtaskmanager API should make sense to someone coming at it cold and obey its users. It has users outside of the TM applet.</li>
</ul></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMAWORKSPACE Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3005" rel="noreferrer">https://phabricator.kde.org/D3005</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>ivan, Plasma, hein<br /><strong>Cc: </strong>plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>