> On 2009-09-11 19:44:22, Aaron Seigo wrote: > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, > > line 366 > > <http://reviewboard.kde.org/r/1370/diff/5/?file=10962#file10962line366> > > > > this is only needed if m_showOnlyRemovable changes, correct? > > wrote: > it is used also to hide or show the devices when you (un)check the option > "show all the items" in the context menu. > > wrote: > yes, that's what i said :) > > wrote: > no, i mean when you right click the dialog, and you set the option to > show or not even devices that normally would be hidden too. like when you > right click on the Places panel in dolphin. In fact resetDevices() is called > in configAccepted() (if m_showOnlyRemovable changes) and in > setAllItemsShown(bool shown) (if m_showAll changes). > > wrote: > ok, let's back up a bit here. > > that call to resetDevices is in DeviceNotifier::configAccepted. so i'm > talking about when the configuration is changed. that means that in > configAccepted, which is what line 365 here is, resetDevices _does not need > to be called_ unless m_showOnlyRemovable changes. make sense? > > now forward again :) > > looking a m_showAll in NotifierView, Show All devices is always in the > context menu and always enabled, even when there is nothing to show. > probably, if there are no items to show there doesn't need to be a context > menu in the view at all. > > i'm still not really sure what the real world use cases for > hiding/showing individual items in the notifier are other than "it's neat > that i can"; i'm trying to imagine someone who has so many items listed there > that it's unmanagable otherwise. or someone who keeps two items in one > notifier widget and has a second notifier widget with some others? hm. > > on the other hand, i can see this resulting in confusion when when > someone plugs a device in, marks it to not be shown, then later plugs it in > (and hopefully it's impossible to have different devices with the same name?) > and it doesn't show up. particularly is days or weeks have passed between > those two events. > > and why would i NOT want my camera to show up when i plug it in? what's > the use case for that? > > this is why i'm still really unsure that turning the device *notifier* > into a device *listing manager* in this way is the best of ideas. > > there's really two similar ideas here it seems: managing devices that are > plugged and unplugged (which may or may not be storage devices) and listing > storage devices. we actually have a widget for both of those tasks, but the > storage device lister doesn't provide access to actions related to the > device, which the device notifier does. > > this really comes back to use cases. can you provide use cases to go with > the hide/show individual items feature? > > oh, and the widget should "emit configNeedsSaving()" after calls to > writeEntry :) > > wrote: > ok, i misunderstood a bit what you wanted to say. > > so, yes. in configAccepted() the call to resetDevices() is needed only if > m_showOnlyRemovable changes, so we could actually check if the variable > changed value and only in the case call resetDevices() and better yet add or > remove the not removable devices without reset all the devices. > > as regards the show/hide system: > saying that Show all devices is always enabled you mean it's always > checked? if it is it is definitely a bug in the patch. i don't think the fact > that the context menu is shown even when there are no devices is a big deal, > but anyway it would be simple to make a check. > anyway it uses the uuid provided by solid and not the names to identify > the devices, so if you have two devices with the same name and you hide one, > the other will remain visible (unless the uuid are equals, but i think this > is a really rare case, if it is possible). > Actually i decided to develop this feature only because a user on > kde-look said to me that the applet was of no use for him because he had lots > of devices and, unless he was able to hide some of them, the plasmoid was > unusable. (comment #7 on > http://kde-look.org/content/show.php?content=106051&forumpage=2). > > wrote: > i still think a better answer is to extend the disk monitor. > > however, as the places view does indeed have this behaviour, for > consistency i suppose the device notifier can/should too. please make it > consistent with the places view (e.g. in dolphin), however, so that the "show > all" entry isn't shown unless there are hidden items to show. > > after that, this patch can go in afaic. > > do you have an svn account? if not, would you like to continue working on > this (and/or other items) in KDE's svn?
ok, i'll do this change and i think this evening i'll send the new revision. no, i don't have an svn account, and yes, i'd like to continue to work on KDE (preferably plasma), but i don't know how much free time i'll have in the future, because on the first of october i'm starting the university - Giulio ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2298 ----------------------------------------------------------- On 2009-09-11 21:32:17, Giulio Camuffo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1370/ > ----------------------------------------------------------- > > (Updated 2009-09-11 21:32:17) > > > Review request for Plasma. > > > Summary > ------- > > This is a patch that modifies quite heavily the behaviour of the Device > Notifier. > It comes from here: > http://kde-look.org/content/show.php/Device+Manager?content=106051 > It can show the not removable devices too, it can mount them automatically or > with a click, since the "eject" button is a "mount" button when the volume is > umounted. So that guy on the dot will be ok. > It can hide some items in the same way as Dolphin's places (hide item/ show > all). > Finally, it shows the various opening actions under the device instead of > calling that xp-ish window. > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h > 1022457 > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.cpp > 1022457 > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h > 1022457 > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp > 1022457 > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt > 1022457 > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui > PRE-CREATION > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h > 1022457 > > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp > 1022457 > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h > 1022457 > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp > 1022457 > > Diff: http://reviewboard.kde.org/r/1370/diff > > > Testing > ------- > > I'm using it every day since I released 0.1 on Kde-look. I tried all the > options on my pc and they work. Some people on kde-look posted some comments > about some problems, but it seems to me they are very particular cases, so in > my opinion it is quite stable to go in trunk, but anyway review it! :) > > > Screenshots > ----------- > > screen > http://reviewboard.kde.org/r/1370/s/183/ > > > Thanks, > > Giulio > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
