GNOME Bugzilla – Bug 343839
Eject button for removeable drives in places sidebar
Last modified: 2008-10-10 16:45:22 UTC
Hello, one thing that really annoys me in Nautilus is that I can't eject a removeable device in the "places" sidebar. You can only do this with the desktop icon, or going to the "Computer" place. A simple and efficient solution would be to add a little eject button to each "places" entry when it is a removable device - similar to how Mc OS X Finder is doing it. This could be done both in the Nautilus places sidebar, and in the places panel menu! Tom
Created attachment 66736 [details] Mockup showing Nautilus with eject buttons This mockup shows how I would add an eject button to removeable devices in the places sidebar - and similar to the places menu. Another suggestion would be to make the device icons a little bigger and print some small information beneath the device's name, like the free space on it.
Hi, thanks for your bug report! First of all - please file only one reqeust per bug. I know it's tempting sometimes, but it makes the bugs _really_ hard to track. So let's have this bug be about an eject icon in the places menu, and nothing else. It sounds feasible, and OSX does it. (The places panel menu though is a different story, you can't really have buttons next to menu items, that'd go agains the HIG and any other GUI guideline.)
sorry for the spam, typo ;)
Created attachment 85065 [details] [review] Preliminary, unfinished, experimental patch. Problems with the patch: * Preliminary, unfinished, experimental, etc. * It modifies NautilusCellRendererPixbufEmblem to emit an "activated" signal when clicked and adds the corresponding public API. Of course, a separate GtkCellRenderer class should be created for the same purpose, but it seemed overkill for such a preliminary, unfinished, experimental, etc. patch. * The eject button doesn't react to mouse enter/leave events; this doesn't tell the user that the button is an active GUI element. * Since I couldn't find a media-eject GTK stock icon, I used GTK_STOCK_REMOVE instead for the CellRenderer pixbuf, so the eject button doesn't look right. (I sent a mail to the GTK list about problems regarding this patch, see http://mail.gnome.org/archives/gtk-list/2007-March/msg00069.html)
*** Bug 440584 has been marked as a duplicate of this bug. ***
Setting patch status as "needs-work" because of the transition away from Gnome-VFS.
Created attachment 113902 [details] Eject button patch working. I'm working in a new patch that works with gio system. I attached a screenshot of the new patch working, but it isn't finished.
Created attachment 113906 [details] [review] Test path of eject action in places sidebar. This is the first version of my patch. Its code is a temporal version to test that works properly. The eject button is in the secondary column, but in the next version it will be in the first column removing the secondary column. And this patch adds a new cell renderer used to draw the new button in places treeview. The cairo code has a little fudge to center the image. I request that someone check this code.
Hi Juan, and thanks for your work. However, I don't think creating a subclass of GtkCellRenderer and drawing with Cairo is the way to go to add this button, as it's overcomplicated and reinventing the wheel :) Gtk+ already has a GtkCellRendererPixbuf which does exactly what is needed here. So, I reworked your patch to make it work with a stock GtkCellRendererPixbuf, works great here.
Created attachment 113936 [details] [review] proposed patch The patch.
Created attachment 113937 [details] screenshot
(In reply to comment #9) > Hi Juan, and thanks for your work. > However, I don't think creating a subclass of GtkCellRenderer and drawing with > Cairo is the way to go to add this button, as it's overcomplicated and > reinventing the wheel :) > Gtk+ already has a GtkCellRendererPixbuf which does exactly what is needed > here. So, I reworked your patch to make it work with a stock > GtkCellRendererPixbuf, works great here. > Yeah you are right. This afternon Juan and I we decided to use the Pixbuf because in the very beginning we created the new render with cairo because we wanted a behavior like if it was a button. Right now we modified the the nautilus-cell-renderer-pixbuf-emblem and we add it the activated signal. As soon as we complete your patch with our. We are going to attach it.
Great Job, from all of you. I've reworked Cosimo's patch to align the eject icon with the items above the separator, but not below it: [ volume a [^] ] [ volume b [^] ] ------------------- [ truncated bookm ] instead of [ volume a ] [ volume b ] ------------------- [ truncated bookm ] i.e. the icons were actually moved out of the view for long bookmark names. I solved it by using distinct text cell renderers for those items above and below the separator. I could commit the patch immediately, but... > As soon as we complete your patch with our. We are going to attach it. ...you seem to prepare another patch. What functionality are you planning to include?
(In reply to comment #12) > Yeah you are right. This afternon Juan and I we decided to use the Pixbuf > because in the very beginning we created the new render with cairo because we > wanted a behavior like if it was a button. > Right now we modified the the nautilus-cell-renderer-pixbuf-emblem and we add > it the activated signal. As soon as we complete your patch with our. We are > going to attach it. There's no need for the renderer to emit any signal when activated, as the "row-activated" signal will be emitted anyway by the GtkTreeView when the pixbuf renderer is clicked/activated.
Looks pretty good. But is it accessible in a) the a11y sense (e.g. can I use an onscreen keyboard to hit the eject button); and b) keynav? (Probably not a huge deal if it isn't; I believe we have a menu entry for eject also in the File menu.)
(In reply to comment #14) > (In reply to comment #12) > > > Yeah you are right. This afternon Juan and I we decided to use the Pixbuf > > because in the very beginning we created the new render with cairo because we > > wanted a behavior like if it was a button. > > Right now we modified the the nautilus-cell-renderer-pixbuf-emblem and we add > > it the activated signal. As soon as we complete your patch with our. We are > > going to attach it. > > There's no need for the renderer to emit any signal when activated, as the > "row-activated" signal will be emitted anyway by the GtkTreeView when the > pixbuf renderer is clicked/activated. > I've tested your patch and the volume doesn't unmount if I click in the icon. am I doing anything wrong?
I have a somewhat similar patch for the treeview. I've developped it against 2.20 though so I need some time to port it to trunk if you find it useful.
(In reply to comment #16) > I've tested your patch and the volume doesn't unmount if I click in the icon. > am I doing anything wrong? This seems very strange; it should work (and it does work perfectly here!). Does right clicking on the volume and selecting unmount/eject from the context menu work for you?
(In reply to comment #18) > (In reply to comment #16) > > I've tested your patch and the volume doesn't unmount if I click in the icon. > > am I doing anything wrong? > > This seems very strange; it should work (and it does work perfectly here!). > Does right clicking on the volume and selecting unmount/eject from the context > menu work for you? > It works. Maybe I did something wrong or maybe it is because I am using nautilus 2.22. Dont't worry about it. If it works for probably I modified something somehow.
(In reply to comment #15) > Looks pretty good. But is it accessible in a) the a11y sense (e.g. can I use an > onscreen keyboard to hit the eject button); and b) keynav? > > (Probably not a huge deal if it isn't; I believe we have a menu entry for eject > also in the File menu.) I don't know about onscreen keyboard accessibility, but I can navigate to the button using the keyboard: the button gains a focus ring around him and I can activate it by pressing spacebar or enter just as a regular button.
(In reply to comment #17) > I have a somewhat similar patch for the treeview. I've developped it against > 2.20 though so I need some time to port it to trunk if you find it useful. Hi Gilles, is your patch for the treeview in Places->Tree? Anyway, I think patches for that should go in separate reports, so if there isn't already one about the feature you're implementing, please file one and attach your patch there.
Created attachment 113978 [details] [review] New eject button patch using nautilus-cell-renderer-pixbuf-emblem Yesterday i worked in a new patch using the last patch send by Cossimo Cecchi, but instead of using gtk-cell-renderer-pixbuf, i used nautilus-cell-renderer-pixbuf-emblem with Ignacio Casal changes. In this version the button is in the same column that contains the other elements. And the signal that launch eject function only works when you click in eject icon. But, this patch has a little problem. If you select other row, and you click in eject button, eject function works propperly but then selected row is the ejected row and nautilus throws error windows reporting that nautilus can't open a inexistent device.
(In reply to comment #22) > But, this patch has a little problem. If you select other row, and you click in > eject button, eject function works propperly but then selected row is the > ejected row and nautilus throws error windows reporting that nautilus can't > open a inexistent device. I believe that's because you are still processing the "row-activated" signal emitted by the treeview, even when you click the renderer. So, if we wanted to go the cell renderer way, I believe you have to block in some way the "row-activated" signal from executing open_selected_bookmark (). Using the cell renderer signal there would be no need to save eject_column in the NautilusPlacesSidebar struct, and you could remove column check in row_activated_callback too. Also, you should always set style properties like xalign and follow-state on the renderer when creating it. Finally, the emblem cell renderer also doesn't seem a good place to put the signal to me, and I'd rather create another GtkCellRendererPixbuf subclass for this. Anyway, this approach seems more complicated to me and not worth the increased complexity. The only advantage I can see with this solution over my patch is that, as discussed on IRC with Ignacio, the separator does not get split into two, but that may be a bug to be fixed in GTK+. Christian, what do you think?
I filed gtk+ bug 541551 with a possible patch attached to fix the separator issue.
ok Cosimo Cecchi, i check your patch. (In the first instance, i moddified your patch to move eject button to the first column, but not works). Your patch works propperly. Now we have to wait until the gtk+ treeview patch gets accepted.
Thanks to everybody, I've committed a patch based on Cosimo's last patch, which I modified as described in comment 13: http://svn.gnome.org/viewvc/nautilus?view=revision&revision=14321 I also added an alt+down keybinding for unmounting. Unfortunately, I can not properly test a11y because it is seems to be somewhat broken on my system. I did not set the "follow-state" property because this makes the icon look ugly when the row of an ejectable volume is focused (selection color overlay). Closing as fixed.
Hello, I really think that this patch should be ported to gnome-panel too, for the "Places" menu entry! Is it possible?
*** Bug 418197 has been marked as a duplicate of this bug. ***
In complement of the current bug, Bug 551524 suggests to grey out volumes that are unmounted clicking the new eject icon (like USB Keys)
How do I install the patch posted on comment #26? I saw the entire script but I don't know what to do with it...