Review Request: basic activity manager

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3780/#review5187
-----------------------------------------------------------

Ship it!


obviously it's a work in progress and has its rough edges, but i think we need to get this in sooner rather than later so we can work on it together in svn and not have you carting around ever larger patches. we're committed to having this in for 4.5 since zooming is gone (huzzah for burning the boats on the beach! ;) so let's just get this in and work on it there. the work so far looks reasonable and like its on the right path.


/dev/null
<http://reviewboard.kde.org/r/3780/#comment4677>

    yes, you can implement itemChange and look for scene changes and then do sth like:
    
    d->corona = qobject_cast<Plasma::Corona *>(scene());



/trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp
<http://reviewboard.kde.org/r/3780/#comment4678>

    perhaps try:
    
    Corona *c = qobject_cast<Plasma::Corona *>(m_activityManager->scene());
    
    if (c) {
        c->removeOffscreenWidget(m_activityManager);
    }
    
    actually, since this is in the shell itself and not an external library, it's even easier:
    
    PlasmaApp::self()->corona()->removeOffscreenWidget(m_activityManager);
    
    :)



/trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp
<http://reviewboard.kde.org/r/3780/#comment4679>

    if we end up with more of these kinds of if/else's, then i agree. if it's just for this one case, it's not worth the effort.



/trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp
<http://reviewboard.kde.org/r/3780/#comment4680>

    does the activity manager really need to care if we don't have a containment? we can get the corona by other means.



/trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.h
<http://reviewboard.kde.org/r/3780/#comment4682>

    i'm on the fence as to whether these belong in PlasmaApp or not. for now, it's ok, i think. but if we end up with more complex (or just "more") code related to activity management in plasma-desktop, then we're probably going to want to split it out into its own class.
    
    for now, let's just see where this takes us.



/trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.cpp
<http://reviewboard.kde.org/r/3780/#comment4681>

    again, not sure we want containments really associated too much with the activity manager. it should be fully self-contained and not need to know which containment it is associated with.
    
    in the case where it is embedded in an existing ControllerWindow (e.g. panel toolbox -> activities), we already have one so we know where to show it and which orientation it should take.
    
    in the case where it is being called up directly via PlasmaApp::showActivityManager, i think it should just always be at the bottom of the screen as a horizontal strip. which means we don't need a containment.
    
    the corona can be gotten from PlasmaApp.


- Aaron


On 2010-04-22 21:06:15, Chani Armitage wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3780/
> -----------------------------------------------------------
> 
> (Updated 2010-04-22 21:06:15)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> this is the beginning of the activity manager.
> it's not very pretty yet; it doesn't have many features yet; but it's a start.
> switching between activities and creating new ones works.
> 
> at the moment "activities" are still just containments; soon this will use the proper activity API instead.
> 
> stuff that's not implemented yet:
> -responding to signals for anything other than added activities
> -showing a pretty thumbnail for each activity
> -start/stop/remove buttons
> -filtering
> 
> 
> Diffs
> -----
> 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /dev/null PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/CMakeLists.txt 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.h 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/desktopview.h 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/desktopview.cpp 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.cpp 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.h 1115355 
>   /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.cpp 1115355 
> 
> Diff: http://reviewboard.kde.org/r/3780/diff
> 
> 
> Testing
> -------
> 
> known issues:
> -removing a containment seems to crash ControllerWindow, because the destructor tries to use the containment's corona. I'm wondering if there's a way to fix this other than having ActivityManager store a pointer to the corona... of course it wouldn't be a problem if we could delete things off the scene without removing them first :/
> -if you make the activitymanager or widgetexplorer go away without clicking the close button, its geometry is wrong the next time it's shown. not sure what's up with that.
> 
> 
> Thanks,
> 
> Chani
> 
>