Review Request: eject icons on devicenotifier

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://mattr.info/r/285/#review294
-----------------------------------------------------------

Ship it!


this is a nice set of feature adds and should imho go in.

there is still work that needs to be done, however, at some point:

* the visual connection between the unmount actions and the entry is very low and the rather large

* perhaps the unmount icon (and other future actions?) should only appear on hover so that there isn't a list of actions, but only actions showing for the currently hovered item?

* we need to provide some consistency for how to display  such items in views.. will post an email to the list about this



/trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp
<http://mattr.info/r/285/#comment213>

    magic numbers ... would be nice to replace with an enum.


- Aaron


On 2008-03-11 16:55:34, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://mattr.info/r/285/
> -----------------------------------------------------------
> 
> (Updated 2008-03-11 16:55:34)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch adds a little eject icon to the right of mounted volumes in the device notifier. clicking on it ejects optical discs and unmounts other removable devices. additionally the devices icons now have the usual mounted overlays.
> 
> 
> This addresses bug 158936.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/itemdelegate.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/itemdelegate.cpp
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/notifierview.cpp
> 
> Diff: http://mattr.info/r/285/diff
> 
> 
> Testing
> -------
> 
> tried to unmount eject some dvds/thumbdrives.
> tried also with the --reverse command line switch it puts the eject column on the left.
> 
> 
> Screenshots
> -----------
> 
> some discs mounted
>   http://mattr.info/r/285/s/34/
> 
> 
> Thanks,
> 
> Marco
> 
>