Introduce a HasLauncher data role.
ClosedPublic

Authored by hein on Apr 19 2017, 11:05 AM.

Details

Summary

This is implemented by TasksModel and allows clients to reliably
query whether a row has an associated launcher. Useful for things
like Pin/Unpin action state or layout decisions.

Previously clients would have to reimplement something like
TaskTools::appsMatch on top of the model, which we absolutely do
not want to do - there should only be one copy of the app matching
logic for consistency's sake. In practice clients would do things
like fetch LauncherUrl and run it by launcherPosition(), which
omits appsMatch's AppId comparision, as an example of such unwated
drift. This approach also avoids the large performance overhead
involved.

The role is not implemented by the single-type or munging tasks
models as it has no use there.

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
Lint Skipped
Unit
Unit Tests Skipped
hein created this revision.Apr 19 2017, 11:05 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 19 2017, 11:05 AM

appsMatch depends on AppId and LauncherURLWithoutIcon

You're handling launcherTasksModel changing, but can we rely on this data being static for a proxyIndex?

If not, we need to connect to TasksModel::dataChanged for those roles and potentially then emit that HasLauncher changed.


Having read your other patch you could argue that you don't need to signal this role as you (currently) only use this role from method calls triggered by other events....but if you take that POV, there's no point doing the launcherListChanged connect.

hein updated this revision to Diff 13631.Apr 20 2017, 10:57 AM

Update HasLauncher when windows or startups update roles evaluated by appsMatch.

davidedmundson accepted this revision.Apr 20 2017, 10:58 AM
This revision is now accepted and ready to land.Apr 20 2017, 10:58 AM
libtaskmanager/tasksmodel.cpp
213

Edit:

Can't your just connect to "this" for the dataChanged

and then you can re-use the topLeft and bottomRight indexes

This revision was automatically updated to reflect the committed changes.
hein added a comment.Apr 20 2017, 11:10 AM

Can't your just connect to "this" for the dataChanged

No, because then I'd also get dataChanged for rows from launcherTasksModel and would have to loop over all indices in the range to ignore them.

It's kind of ugly either way. The applet code will currently re-layout everything anyway, whether it's one row changing the role or all of them.