Review Request: eject icons on devicenotifier

[Marco Martin]

> On 2008-03-13 11:41:53, Aaron Seigo wrote:
> > 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
> >

probably should look the same as the kickoff listview
i did put the icon on the right because it seemed to look nicer than after the item, but maybe if there will be more than one action after the item would be better
if the enum instead of magic numers revision is ok i think i would be more confortable to commit as is now (functionality-wise doesn't seem to have problems) and work on the look of the listview in a second time


> On 2008-03-13 11:41:53, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/devicenotifier/devicenotifier.cpp, line 421
> > <http://mattr.info/r/285/diff/1/#file1133line421>
> >
> >     magic numbers ... would be nice to replace with an enum.

added a new role, so now the first column is a OpenAction and the second is a DeactivateAction (more generic than unmoun), so if there will be other actions just add fields to ColumnScope


- Marco


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


On 2008-03-13 13:18:00, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://mattr.info/r/285/
> -----------------------------------------------------------
> 
> (Updated 2008-03-13 13:18:00)
> 
> 
> 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
> 
>