----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1370/#review2133 -----------------------------------------------------------
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. - Aaron 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
