Review Request: make kickoff use popupApplet

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/132/#review120
-----------------------------------------------------------


some initial thoughts on the patch; i like the direction, and the resize handle will be useful in other places, such as the preview applet which has a rather poor implementation of the same thing right now.


/trunk/KDE/kdebase/workspace/libs/plasma/dialog.h
<http://reviewboard.vidsolbach.de/r/132/#comment87>

    this should be in the .cpp file



/trunk/KDE/kdebase/workspace/libs/plasma/dialog.h
<http://reviewboard.vidsolbach.de/r/132/#comment88>

    this probably belongs in the Plasma namespace as generalized (ResizeHandlePlacement is a bit specific for what's really a set of generic compass points)



/trunk/KDE/kdebase/workspace/libs/plasma/dialog.cpp
<http://reviewboard.vidsolbach.de/r/132/#comment91>

    while this default makes sense right now, i wonder if putting it in the more traditional SouthEast corner will end up being more sensible in the general case? we'll have to keep our eye on this as other things use it and see what the pattern is ...



/trunk/KDE/kdebase/workspace/libs/plasma/dialog.cpp
<http://reviewboard.vidsolbach.de/r/132/#comment90>

    we should get some nice art from the developers for this i think, something that goes with the dialog svg itself (perhaps in the same svg even?) with a fall back to CE_SizeGrip if it doesn't exist.



/trunk/KDE/kdebase/workspace/libs/plasma/plasma.h
<http://reviewboard.vidsolbach.de/r/132/#comment92>

    the entries in this enumeration are rather confusing (BottomRightPopup vs RightBottomPopup?) .. would it make more sense to use the compass positions here again? e.g. NorthWest, SouthEast, etc.?



/trunk/KDE/kdebase/workspace/libs/plasma/popupapplet.cpp
<http://reviewboard.vidsolbach.de/r/132/#comment89>

    it's a bit dangerous to write into the applet config group with general keys such as "Height" and "Width". geometry management is usually done by the Applet class itself anyways, so this looks very odd in general.
    
    what i'd recommend doing instead here is creating a sub-group (PopupDialog?) and storing these settings in there.


- Aaron


On 2008-08-10 14:21:31, Loic Marteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/132/
> -----------------------------------------------------------
> 
> (Updated 2008-08-10 14:21:31)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> hello !
> 
> Here is a big patch to make kickoff use popupApplet.
> 
> I have add some code to dialog to let the user resize a dialog .
> The dialog resize handle is activated by giving to the dialog which cardinal direction we want to see it, perhaps we can add support to combine different location later. 
> 
> I have add some code to popupApplet too to let the applet notified when popup is activated and where is located the popup relatively to the icon
> The popup Location is an enum in Plasma. This stuff is to permit fitt's law optimisation.
> 
> The majority of the code i have added in dialog and popupapplet is inspired from the kickoff one.
> 
> There is things missings in the patch but i want to know if the direction is good.
> - Default kickoff size does not work well
> - Tool tip manager does not work
> - the resize handle widget is a little ugly
> - More work to adjust the popup Position in popupApplet is needed to deal correctly with centered icons in panels and to let applets say to popup what it is their preferred alignment.
> 
> Cheers
> 
> 
> Hope than you spend good time at akademy !
> 
> Lo
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/libs/plasma/dialog.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/dialog.cpp
>   /trunk/KDE/kdebase/workspace/libs/plasma/plasma.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/popupapplet.h
>   /trunk/KDE/kdebase/workspace/libs/plasma/popupapplet.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/132/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Loic
> 
>