New devicenotifier moved to kdereview

[Aaron J. Seigo]

On October 4, 2009, Jacopo De Simoi wrote:
>  now, we believe that it is ready for merging into trunk. Of course we need
>  your feedback first, so grab it, try it out and tell us what you think!

with options out of the way :) some thoughts on the plasmoid itself:

* it still says "Devices recently plugged in:" when there are no devices 
listed. that's always seemed a bit odd. it should probably be replaced with an 
applicable label and the divider line removed in that case

* i really like how it's one click for one action, click to expand for the 
multi action case. it just feels "right" now .. nicely done! :)

* when an item is expanded, should the background remain painted? it might 
make it more evident that the item is "open", and it would also allow a way to 
solve the next point, too. (and if the background remains, perhaps the 
capacity bar should too?)

* each DeviceItem gets its own ItemBackground in case it has multiple actions. 
it would be nice if they could share the one ItemBackground between them as 
only one at a time can be shown anyways so anything more seems like a waste; 
it would also allow the hover effect to track with the mouse more effectively 
when there are multiple DeviceItems in the list. if the background remains 
painted on open devices, then the item created by NotifierDialog could be 
shared by all the items rather easily.

* when there is more than one action, and an action is selection, perhaps the 
device item should collapse back to closed. this does two things: it gives 
some immediate feedback that an action was selected, it saves a second click 
needed to collapse the item again. the assumption here is that the click 
action is almost always needed once, which seems to fit the use cases pretty 
clearly. for a single device item this might not matter, but if there are 
multiple devices this could help avoid having to scroll more than needed or 
click around a lot. 

* in DeviceItem there is this line:

    Plasma::IconWidget *actionItem = new Plasma::IconWidget();

where does this object get deleted?

* in NotifierDialog there is a Plasma::Label ("category") and a 
Plasma::Separator ("separator") created in 
NotifierDialog::searchOrCreateDeviceCategory. they are deleted in 
removeDevice, but that is only called when the source from the DataEngine goes 
away, the device is hidden by the user or resetDevices is called (e.g. after 
configuration). it seems these will leak if the applet is simply closed

* there is another separator created in NotifierDialog::buildDialog that has 
no parent. 

* i committed a work around for a Qt bug that caused the capacity bar to 
sometimes be shown in the wrong place (QGraphicsLayout strikes again!)

nothing too big, really, and some nice improvements over what was there.

-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20091004/8480a88d/attachment.sig