On Sunday 04 October 2009 20:57:11 Aaron J. Seigo wrote: > 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
The divider line was put there to avoid the bad "cropped" feeling of the scrollwidget when scrolled down; this is actually being addressed in the scrollwidget itself, with the nice shadows that now appear when the scrollbar is visible, so I believe that it can go back to having it getting along with the categories. Still I don't know if it makes sense to have one also for the first category. > * 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?) The background could in principle stay, for the capacity bar there is an issue: there is no way to make a refresh signal "free space changed"; that's why we show it only on hover (and for the same reason it is like that for kfileplacesitem); > * 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. I don't think I understand this; if we had one itemBackground shared by all items we would see the selection bar move from a device to another one. Do I understand correctly? > * 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. Makes sense! > > * in DeviceItem there is this line: > > Plasma::IconWidget *actionItem = new Plasma::IconWidget(); > > where does this object get deleted? good point. > * 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 right. > > * 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!) Excellent, thanks. --J _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel