Review Request: big revamp of Device Notifier

[Giulio Camuffo]

> On 2009-09-11 19:44:22, Aaron Seigo wrote:
> > is the latest version the most recent available version of the patch? because it doesn't apply cleanly to trunk.
> > 
> > anyways ... i really don't like the "how long to show the popup" configuration item and the other 2 config items do need seem linguistic and layout adjustments, but that's easy to do once this is in. "how long" could remain configurable in the config file, but i don't think it belongs in the UI; that's just an excuse not to make a good default.
> > 
> > having to click twice for items with one action is not great. instead, when there is just one item how about just putting the action description as the sub-title text for the main item and launch it on click/select? so it would be:
> > 
> > ICON Camera
> > ICON Open with Digikam...
> > 
> > in the case of multiple items, it could look like:
> > 
> > ICON USB Storage Device
> > ICON 2 available actions ...
> > 
> > and when it's clicked then the options show up. it could happen on hover, but i'm not sure that makes sense in this case.
> > 
> > if the options could also be made smaller that would also help show that these are "detail items that belong to the item above" better IMO; the icon should probably only need to be roughly as tall as the line of text?
> > 
> >
> 
> Giulio Camuffo wrote:
>     The problem is that the subtitle could be covered by the KCapacityBar. So we need to find a solution, as to show the actions on hover.
> 
> Aaron Seigo wrote:
>     the capacity bar should be painted below any text.

that would solve all, but personally i dislike very much from an aesthetically pov the items as they are painted in the current device notifier, with the increased height. i find them fat. and if the volumes are umounted there would be no capacity bar, and that would result in a lot of wasted white space


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

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


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