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
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