Review Request: big revamp of Device Notifier

[Giulio Camuffo]

> On 2009-08-24 12:43:32, Aaron Seigo wrote:
> > i'm not going to look at the code just yet. instead i'd like to focus on what's exposed in the configuration dialog first. then we can get to code :)
> > 
> > automounting: that's already been discussed on the mailing list so we can skip that one :)
> > 
> > show only removable devices: why would we want to show non-removable devices here?
> > 
> > show popup when device is inserted: what is the benefit to this that a configuration option is worth while?
> > 
> > actions layout: this is a very technical entry, e.g. "what is an action?" can we just put them in a vertical list, like the rest of the widget, and be done with it? lists with a single dimension to them tend to allow for better visual scanning in most cases anyways.
> > 
> > when inserting a device show popup for: this really belongs with the "show popup when" option (so should be moved closer to it :) and the "0" for never is redundant to the show popup option isn't it? anyways, what is the benefit to this? would it make more sense to have a sensible default and just autohide it at some point? is there really a great benefit to being able to tweak it down to the second?
> > 
> > i REALLY like how this gets rid of the ugly old-school popup dialog when there is more than one option, so there is real value to this patch and i think it should go into svn at some point. however, i don't currently see the benefits to any of the configuration options provided here. keeping widgets simple and configuration down to just what is really beneficial is what we should be striving for, otherwise plasma-desktop becomes clumsy to use and the code becomes more difficult to maintain.


> show only removable devices: why would we want to show non-removable devices here?
>
I find it really useful to be able to have them shown too: I don't use the automount, and I often use the console to browse through the directories. So i need to mount easily the partitions I need to access to without having to open dolphin and go to the place, or mount manually in console. Actually this is maybe the main thing that pushed me to develop it, and I find 
a bit annoying that the dataengine doesn't shows the partitions managed by fstab too.

> show popup when device is inserted: what is the benefit to this that a configuration option is worth while?
>
I'm not an usability expert, I simply thought that maybe some people find the 
popup annoying.

> actions layout: this is a very technical entry, e.g. "what is an action?" can we just put them in a vertical list, like the
> rest of the widget, and be done with it? lists with a single dimension to them tend to allow for better visual scanning in 
> most cases anyways.
>
I like better the vertical listing too, but if the actions are many, and the 
devices too, the applet could get really long and uncomfortable to use.

> when inserting a device show popup for: this really belongs with the "show popup when" option (so should be moved closer to
> it :) and the "0" for never is redundant to the show popup option isn't it? anyways, what is the benefit to this? would it 
> make more sense to have a sensible default and just autohide it at some point? is there really a great benefit to being
> able to tweak it down to the second?
>
The 0 means that it won't hide until you hide it manually, I know I'm not able to make interfaces :). Maybe it is not useful to be able to tweak the seconds, but I think we could keep the ability to autohide or not.


- Giulio


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1370/#review2133
-----------------------------------------------------------


On 2009-08-21 19:26:43, Giulio Camuffo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1370/
> -----------------------------------------------------------
> 
> (Updated 2009-08-21 19:26:43)
> 
> 
> 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/devicespaceinfodelegate.cpp 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicespaceinfodelegate.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/configurationpage.ui PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/CMakeLists.txt 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierdialog.cpp 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h 1013960 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp 1013960 
> 
> 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
> 
>