GNOME Bugzilla – Bug 754513
Dragging files to the sidebar makes the "+new bookmark" row to show up
Last modified: 2017-09-18 19:41:49 UTC
It should appear only when draggin dirs.
(In reply to Lapo Calamandrei from comment #0) > It should appear only when draggin dirs. Hi, I was trying to implement this feature but found that 'New bookmark' row is added by function gtk_places_sidebar_set_drop_targets_visible(). Correct me if I am wrong. https://developer.gnome.org/gtk3/stable/GtkPlacesSidebar.html#gtk-places-sidebar-set-drop-targets-visible
(In reply to rishabh from comment #1) > (In reply to Lapo Calamandrei from comment #0) > > It should appear only when draggin dirs. > > Hi, I was trying to implement this feature but found that 'New bookmark' row > is added by function gtk_places_sidebar_set_drop_targets_visible(). Correct > me if I am wrong. > > https://developer.gnome.org/gtk3/stable/GtkPlacesSidebar.html#gtk-places- > sidebar-set-drop-targets-visible Yes, however you have the drag context as a parameter, so you can ask the application using that function if the source object is a file or folder.
any news on this one?
For newcomers with some experience already.
Hello, I was trying out the
Hello, I was trying that out, and it seems that it be done easily on the reciever end.(gtk places side bar)(In reply to Carlos Soriano from comment #2) > (In reply to rishabh from comment #1) > > (In reply to Lapo Calamandrei from comment #0) > > > It should appear only when draggin dirs. > > > > Hi, I was trying to implement this feature but found that 'New bookmark' row > > is added by function gtk_places_sidebar_set_drop_targets_visible(). Correct > > me if I am wrong. > > > > https://developer.gnome.org/gtk3/stable/GtkPlacesSidebar.html#gtk-places- > > sidebar-set-drop-targets-visible > > Yes, however you have the drag context as a parameter, so you can ask the > application using that function if the source object is a file or folder. I was looking around. Which functions is available for getting the selection list from a GdkDragContext?
(In reply to Akilan Elango from comment #6) > Hello, > I was trying that out, and it seems that it be done easily on the reciever > end.(gtk places side bar)(In reply to Carlos Soriano from comment #2) > > (In reply to rishabh from comment #1) > > > (In reply to Lapo Calamandrei from comment #0) > > > > It should appear only when draggin dirs. > > > > > > Hi, I was trying to implement this feature but found that 'New bookmark' row > > > is added by function gtk_places_sidebar_set_drop_targets_visible(). Correct > > > me if I am wrong. > > > > > > https://developer.gnome.org/gtk3/stable/GtkPlacesSidebar.html#gtk-places- > > > sidebar-set-drop-targets-visible > > > > Yes, however you have the drag context as a parameter, so you can ask the > > application using that function if the source object is a file or folder. > > I was looking around. Which functions is available for getting the selection > list from a GdkDragContext? In this case I believe you should just avoid to call gtk_places_sidebar_set_drop_targets_visible (TRUE) from the nautilus side, since we can know if it's a file or a folder just when starting the dnd. In case the mouse is hovering the sidebar itself, it will go through drag_motion_callback in the gtkplacessidebar, and try to get the drag data with gtk_drag_get_data which will return a list of GFiles where you can see if one of them is a file instead than a directory and therefore not show the new bookmark row. Sadly, I was mistaken and this is not for newcomers, since it requires changes in gtkplacessidebar too.
diff --git a/gtk/gtkdnd.c b/gtk/gtkdnd.c index 608dfaa30d..4601064ca2 100644 --- a/gtk/gtkdnd.c +++ b/gtk/gtkdnd.c @@ -635,6 +635,18 @@ gtk_drag_get_data (GtkWidget *widget, g_return_if_fail (GTK_IS_WIDGET (widget)); g_return_if_fail (GDK_IS_DRAG_CONTEXT (context)); + char *fileuri; + GFile *f; + for (GList *B = gdk_drag_context_list_targets (context); B != NULL; B = l->next) + { + f = G_FILE(B->data) + file_uri = g_file_get_uri(f) + if (g_file_test(f,G_FILE_TEST_IS_REGULAR)) + { + return; + } + } + selection_widget = gtk_drag_get_ipc_widget (widget); g_object_ref (context); Will this work in the gtk part?
Or should I use gdk_drag_get_selection?
(In reply to Vyas Giridhar from comment #9) > Or should I use gdk_drag_get_selection? I think so, yeah. I need to double check, but we should use the dnd API when possible.
Created attachment 357559 [details] [review] dnd: make sidebar show "new bookmark" only for folders Make sure sidebar shows "new bookmark" only when folders are being dragged. https://bugzilla.gnome.org/show_bug.cgi?id=754513
(In reply to Nelson Benitez from comment #11) > Created attachment 357559 [details] [review] [review] > dnd: make sidebar show "new bookmark" only for folders > > > Make sure sidebar shows "new bookmark" only when folders > are being dragged. > > https://bugzilla.gnome.org/show_bug.cgi?id=754513 This patch fixes the issue by just checking the items being dragged if they are all folders. This patch fixes the issue in Nautilus, but would be nice to also have the fixes for GtkPlacesSidebar and Filechooser that I posted on bug 762252 (which included this nautilus patch too).
Review of attachment 357559 [details] [review]: Good progress, some review: ::: src/nautilus-dnd.c @@ +701,3 @@ + * are folders, FALSE otherwise */ +gboolean +nautilus_drag_items_are_all_folders (const GList *list) not sure a separated function to know if all drag items are folders is necesary. I can imagine having a general "all files are folders" function inside nautilus-file-utilities, then sending to that your list. I prefer to do a O(2n) method, one that converts to a list of files then one that checks if all of those are folders, rather than this. We should avoid adding this half-random API, if the cost of it technical wise is not that much. @@ +707,3 @@ + { + NautilusDragSelectionItem *item = l->data; +{ remove extra space after ! ::: src/nautilus-window.c @@ +1184,3 @@ nautilus_window_start_dnd (NautilusWindow *window, + GdkDragContext *context, + gboolean only_folders) if the code is just doing nothing if one of the paremeters is TRUE, it doesn't make sense to have it in the first place. @@ +1191,3 @@ + if (only_folders) + { /* Items being dragged are only folders, so allow places sidebar show "New bookmark" */ I think the comment is unnecesary, since only_folders is already a boolean inside an if, and the API of gtkplacessidebar is precisely "show drop targets visible", so the code already explains that comment. It doesn't say new bookmark only in the API in pourpose, so it's good to keep it aligned with that.
Created attachment 357813 [details] [review] Updated patch Updated patch with review comments, except for: > I can imagine having a general "all files are folders" function inside nautilus-file-utilities" I made this "all files are folders" function be in nautilus-file.c instead of nautilus-file-utilities.c. As the function iterates over a list of NautilusFile I thought was more natural place.
Review of attachment 357813 [details] [review]: Excellent, thanks Nelson!! ::: src/nautilus-file.c @@ +4211,3 @@ + const GList *l; + + for (l = files; l != NULL; l = l->next) I pushed with a follow up with a nitpick fix, the style of nautilus is 4 spaces.
git bz failed. The commits are: https://git.gnome.org/browse/nautilus/commit/?id=f0c81e22a3e9c3cbc1a70245e629caa2209c75ba https://git.gnome.org/browse/nautilus/commit/?id=1f1da4e495908cf1adb6f7f06659a3c59e851334
(In reply to Carlos Soriano from comment #16) > git bz failed. The commits are: > https://git.gnome.org/browse/nautilus/commit/ > ?id=f0c81e22a3e9c3cbc1a70245e629caa2209c75ba > https://git.gnome.org/browse/nautilus/commit/ > ?id=1f1da4e495908cf1adb6f7f06659a3c59e851334 Thank you!
not fixed in nautilus 3.25.92 on ubuntu 17.10.
*** Bug 762252 has been marked as a duplicate of this bug. ***
Reopening because it's not fixed on Arch.
(In reply to Strangiato from comment #20) > Reopening because it's not fixed on Arch. Can you share a screenshot of it?
Created attachment 359993 [details] bug is still happening on Arch, nautilus 3.26 Sure.
On nautilus-3.26.0-1.fc27.x86_64, “New Bookmark” appears when you start dragging a folder, but also when you drag a file *over* the sidebar.
can you confirm it does not appear when not in the sidebar?
It shows a placesholder (New Bookmark) but nothing happens when I drag it all the way.
Created attachment 360011 [details] Dragging a file in Nautilus 3.26
Created attachment 360012 [details] Dragging a file over the sidebar in Nautilus 3.26
(In reply to Strangiato from comment #22) > Created attachment 359993 [details] > bug is still happening on Arch, nautilus 3.26 > > Sure. Please re-read my comment 12, the patch committed in this bug it's for Nautilus only, the fix for gtkplacessidebar (a gtk widget nautilus uses as his sidebar) is on bug 762252 along with a patch for filechooser also, those patches are still waiting in their bugs. Please also remove bug 762252 being a dup of this, instead you could mark it as blocking this, they share the same problem but on different components and with separate patches.