introduce candidateContainments
AbandonedPublic

Authored by mart on Mar 14 2018, 5:23 PM.

Details

Summary

DesktopView contains a property candidateContainments which are all containments with the same screen for all activities.
Plasma mobile can use it to do activity switching with a swipe gesture

depends from D11361

Test Plan

works well on plasma mobile, misused it by purpose, it doesn't crash

Diff Detail

Repository
R120 Plasma Workspace
Branch
mart/containmentPreview
Lint
No Linters Available
Unit
No Unit Test Coverage
mart created this revision.Mar 14 2018, 5:23 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 14 2018, 5:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart requested review of this revision.Mar 14 2018, 5:23 PM
mart changed the visibility from "Public (No Login Required)" to "No One".Mar 14 2018, 5:23 PM
mart edited the test plan for this revision. (Show Details)
mart added a comment.Mar 14 2018, 5:26 PM

switching activities by swipe in plasma mobile shell, the rectangle in the center has the activity name

mart changed the visibility from "No One" to "Public (No Login Required)".Mar 14 2018, 5:26 PM
davidedmundson requested changes to this revision.Mar 14 2018, 5:57 PM
davidedmundson added a subscriber: davidedmundson.

ShellCorona only creates the containment in currentActivityChanged, so this swiping stuff isn't going to work until you've first switched containment an existing way, then started swiping.

shell/desktopview.cpp
265

In the main code path shellcorona is responsible for assigning containments to desktopviews.

In this code path DesktopView is extracting the relevant containments for itself.

That's super messy, please redesign.

shell/shellcorona.cpp
1239

This is generically finding a containment.

The method name and comment shouldn't have anything to do with what 1 usage of it is doing.

This revision now requires changes to proceed.Mar 14 2018, 5:57 PM
mart added a comment.Mar 15 2018, 10:31 AM

ShellCorona only creates the containment in currentActivityChanged, so this swiping stuff isn't going to work until you've first switched containment an existing way, then started swiping.

no, the Containment* instances are all created immediately, as well the contained ContainmentInterface*
What's created on demand (that is heavy indeed) is the qml stuff, which is created when the ContainmentInterface* gets a view, which can be controlled by the qml part of the shell (ie, when to cann this function and reparent the resulting ContainmentInterface*)

mart added inline comments.Mar 15 2018, 10:36 AM
shell/desktopview.cpp
265

shellcorona assigns what is actually the current one, which is conceptually another thing.
not sure how to go on the other direction..
perhaps the corona could assign as well as another property like "allcontainmentsForScreen" which is then.. a model which has activity id, name and pointer to containment?

mart updated this revision to Diff 29595.Mar 15 2018, 1:43 PM

approach with a model

a possible approach is this: a model with all the activities that map to
items, but i don't like this, because it's pretty much duplicating the
activity model present in kactivities with a worse version.

another approach i'll try shortly, is to just have a list property
of all possible containments and then it's a problem of the qml part to
find the right ones for the right activities

mart added a comment.Mar 15 2018, 1:43 PM

a possible approach is this: a model with all the activities that map to
items, but i don't like this, because it's pretty much duplicating the
activity model present in kactivities with a worse version.

another approach i'll try shortly, is to just have a list property
of all possible containments and then it's a problem of the qml part to
find the right ones for the right activities

mart retitled this revision from introduce the function containmentGraphicsItemPreview to introduce candidateContainments.Mar 15 2018, 6:37 PM
mart edited the summary of this revision. (Show Details)
broulik added inline comments.
shell/desktopview.cpp
55

Remove or cleanup

93

Use initializer list

shell/desktopview.h
41

use Qt::DisplayRole for that

45

You're not using those from QML (not needed for model magic property)

shell/shellcorona.cpp
1228

const

1571

qAsConst?

mart abandoned this revision.Apr 19 2018, 2:30 PM