Re: Review Request: basic activity manager
On April 27, 2010, Chani wrote: do I really need to bother setting the screen at all? or can I assume it'll magicallly go to the Right one? it may not always go magically to the Right One(tm), but let's try it out as an assumption and see how well the window manager performs for us. if I do need to set the screen, what's the best thing to set it to? the current screen (if something can tell me that)? you can get which screen the mouse is on by taking the current mouse cursor position and asking which screen that point belongs to. aka how krunner picks a screen -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: basic activity manager
/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. the most irritating use of m_containment is to place the view on its screen and desktop. the widgetexplorer probably should still use this to be sure that doubleclicking an applet sends it to the containment expected (especially with view-per-desktop)... but this isn't so important for the activity manager. do I really need to bother setting the screen at all? or can I assume it'll magicallly go to the Right one? if I do need to set the screen, what's the best thing to set it to? the current screen (if something can tell me that)? -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: basic activity manager
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3780/#review5180 --- First, I'm getting two compilation problems - AbstractIcon and AbstractIconList are not in Plasma namespace. The other thing is that AbstractIcon and AbstractIconList don't have PLASMAGENERICSHELL_EXPORT so the linker fails as well. Did you make some patches to libs/plasmagenericshell as well? - Ivan 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: basic activity manager
--- 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_castPlasma::Corona *(scene()); /trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp http://reviewboard.kde.org/r/3780/#comment4678 perhaps try: Corona *c = qobject_castPlasma::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
Re: Review Request: basic activity manager
On 2010-04-23 16:33:26, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/desktop/shell/controllerwindow.cpp, lines 312-314 http://reviewboard.kde.org/r/3780/diff/1/?file=24331#file24331line312 does the activity manager really need to care if we don't have a containment? we can get the corona by other means. I think it mattered for positioning the window properly... either that or it's an artifact of the widgetexplorer On 2010-04-23 16:33:26, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.h, lines 123-131 http://reviewboard.kde.org/r/3780/diff/1/?file=24335#file24335line123 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. ahh. yeah, I asked if it belonged in plasmaapp or desktopcorona iirc, and you didn't say anything, so I just started adding to plasmaapp. we *are* going to end up with more codde; this is the bare minimum for whats implenented so far. I also think i'll end up with a class representing a single activity, it'll make things easier On 2010-04-23 16:33:26, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.cpp, line 471 http://reviewboard.kde.org/r/3780/diff/1/?file=24336#file24336line471 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. hmm. I'll look into that next then, see how many contaimnent assumptions aree in the code - Chani --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3780/#review5187 --- 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 ___ Plasma-devel mailing list Plasma-devel@kde.org
Re: Review Request: basic activity manager
On 2010-04-23 08:20:21, Ivan Cukic wrote: First, I'm getting two compilation problems - AbstractIcon and AbstractIconList are not in Plasma namespace. The other thing is that AbstractIcon and AbstractIconList don't have PLASMAGENERICSHELL_EXPORT so the linker fails as well. Did you make some patches to libs/plasmagenericshell as well? (hmm, my comment seems to have vanished) yes, forgot those patches, sorry. - Chani --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3780/#review5180 --- 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: basic activity manager
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3780/ --- 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel