GNOME Bugzilla – Bug 73240
GtkNotebook should have drag-and-drop support
Last modified: 2011-02-04 16:10:23 UTC
Notebook should support an API for dragging tabs between notebooks, and reordering tabs in the notebook. This would be used for IDE-style MDI, tabbed apps like browsers/terminals, etc. The minimum support would be a tab_dropped signal, a way to enable DND, and a way to specify which notebooks are in a "tab exchange group" and can accept tabs from each other.
Just to try to avoid duplicated work, I mention here I started to work on something like that (I didn't read your bug report and came up with mostly the same requirements). I'd add to these requirements that it should be able to do "something" when a tab is dropped outside of an app window (eg on the desktop). I'd do that by letting the user specify a "create_new_notebook" callback which is called when a tab is dropped outside of the app. Alternatively, it might be possible to handle that with the "tab_dropped" signal. Concerning the control of DND, it should be possible to choose between no dnd, dnd within notebook, dnd between several notebooks. Atm the code I'm attaching is quite hacky (I intend to clean it up a little), but it already is able to drag and drop tabs between notebooks in a few apps (it works with gedit, will work with galeon when some fixes are made on galeon side, but doesn't work yet with profterm) This code is also lacking the signal emission atm, and also the various configuration possibility evoked by Havoc. I'm attaching the code here, even if I feel I should keep it hidden until I clean it a little :) If you want me to post updates to this code here, please let me know
Created attachment 10851 [details] preliminary version of a notebook with dragndrop support
I just want to point out that a common implementation of this is quite interesting. This would e.g. help applications like Galeon that implement this by themselves, would lead to a consistent handling of this, ensure that a11y issues are taken care of, and would application programmers to easily enable this. (Think of e.g. gnome-terminal.)
*** Bug 122223 has been marked as a duplicate of this bug. ***
What user interface design are you thinking of employing? For example, in Gaim, you see an insertion pointer as you drag the tab, though the subject tab stays until you drop it somewhere. Whereas in Epiphany, the tab is moved in real time as you move it. I vote for the Gaim way, because I think it's more clear. And with Epiphany it's easy to mistakenly drop the subject tab into a new window of its own. This happens if you may drop it beneath the tab row. With Gaim you would notice such a mistake, because then the insertion pointer wouldn't show.
I think the Epiphany method is better in principle, since it provides more direct feedback. However: it would be even better and easier if the tabs would slide smoothly from position when dragging or inserting them, like for instance Epiphany toolbar items do. The instantaneous switching of position makes it harder to track with the human eye where the change is happening.
Still valid for latest GTK.
Created attachment 56045 [details] [review] proposed patch, first draft This patch implements drag and drop in the same GtkNotebook, it adds the following API: - a "reorderable" property - gtk_notebook_[sg]et_reorderable() a "tabs-reordered" signal when the tabs positions change would be desirable, but still not implemented. Regarding the dropping outside (or inside other tabs), it would be nice to add a "detachable" property, but that's quite harder to do, and maybe (IMHO) outside the scope of a supposedly simple notebook widget
err, the last paragraph was supposed to say "... dropping outside the notebook (or inside other notebooks)..." sorry for the spam
some comments from looking over the patch (i'll try it later tonight) - i think a simple approach to detaching might be to just emit a signal with the ppointer and page number when the drop happens outside the notebook - is there any visual feedback during the drag ? (i guess i should just try it) possible approaches to feedback would be just a drag icon (firefox does it like this), or something like the column drag in gtktreeview or evolutions table widget. - we need this to be accessible with the keyboard. its probably worth looking at the key combinations used for column reordering in gtktreeview
ok, I tried it now. Works relatively nicely, with visual feedback similar to the one in GtkTreeView. One additional idea to consider: should reorderable be a child property of the tabs, like it is for treeview columns ?
I didn't see a way to get notified about the tab reordering; IMHO there should be a "tabs-reordered" signal emitted when this happens.
Matthias: - You're right regarding using a child property, just for consistency sake - I'll have a look too at the keybindings issue - Regardless of having a signal for dropping outside the notebook, maybe should we add another child property to enable "detachability"? without it enabled the notebook should be permissive with the drop point (i.e: pretty much like the Gtk[VH]Scale). This patch already implements such behavior. Chpe: In my first comment I told that it's still unimplemented :) I'll try to work on it all this weekend
Ok, the first impression from other people in the office is that they like the general visual feedback. Of course, allowing detaching by dragging is a bit harder with this kind of feedback, because you have to allow the "shake loose" the tab label somehow. There is a little visual defect when dragging a label all the way to the edge in non-scrolled mode.
Created attachment 57034 [details] [review] proposed patch, second draft Matthias, I've tried to detect the visual defect you talked about, but can't see anything... which kind of visual defect are we talking about? :/ Other issues about this patch: - Implements the "reorderable" and "detachable" child properties. - Uses the same keybindings than GtkTreeView for column reordering - Implements the "tab_reordered" and "tab_detached" signals - In these signals, unlike in "switch_page", the second argument is the child widget, not the GtkNotebookPage containing it - Leaves only one padding pointer in GtkNotebookClass Any comment is welcome
I've tried this patch, and have a few comments: - it's a bit weird that 'reorderable' is a child property for each tab instead of global, esp. since if you for example have tab 0 reorderable, tab 1 non-reorderable you can still drag tab 0 after tab 1 and have in effect changed the position of tab 1! Also I just don't see any use cases for some-tabs-reorderable-some-not ? - if there's room left to the right of the tabs, you can drag the tab all the way to the end of the notebook, but releasing will just make it the last tab. IMHO it shouldn't follow the mouse beyond the last tab position. - do you plan to make drag-to-another-notebook possible for detachable tabs?
Christian, its the same with reorderable treeview columns. I agree that it is not very useful. It does have some effect though: you can't start a drag on a column header thats not reorderable, and you can't change the relative positioning of two non-reorderable columns wrt to each other (though you can drop stuff in between). Maybe you are right though, there is no use case, and consistency with the tree view is not very important here. Carlos, the visual defect we saw was that if you can drag a tab label all the way to the left or right, part of it vanishes outside the area of the notebook. I'll try your second patch now...
Hmm, so the detachable thing seems totally undiscoverable. We need to add some (large) threshold distance and visually detach the tab and make it jump to the mouse to make it clear that the tab is detached. I still the the "visual defect" I tried to explain in the last comment.
*** Bug 326764 has been marked as a duplicate of this bug. ***
Just something about the cursor mutation when drag n dropping. (I see it in Gajim where MOVE icon is used for dnd tabs) It show a "move file" icon, it may be confusing about what it really do when people see that. I see a "closed hand" icon (as if we catch something) used in Epiphany when drag n drop tabs that show better what we do (catch tab, move it, and let it fall where we decide).
Created attachment 58161 [details] [review] proposed patch, third draft This patch implements visual DnD when detaching tabs, handles quite better the drag threshold and implements Christian's suggestion about not moving tabs past the last position when reordering
Oh, and also found a possible cause for the visual defect, anyway, limiting the tabs movement should have minimized it a lot, if not removed.
I think this really looks and feels great now. The one thing it is missing now is a way to undo the tearoff without ending the drag. Ie I want the tab to snap back in the notebook when I get close to the tab row.
Carlos, one other thing that would be great to have is a testcase demonstrating dnd between notebooks and/or to the desktop.
Thinking a bit more about how detaching is currently implemented, maybe it's so simple that will require gross hacks on the api users' side (and wouldn't even be suitable for DnD between several processes, nor for detecting the tab position below the pointer). To improve this situation, something like the following could be implemented: * Functions to define a notebook group: void gtk_notebook_set_group_name (GtkNotebook*, gchar*); gchar *gtk_notebook_get_group_name (GtkNotebook*); GtkNotebook would use these groups names to know which tabs may interchange tabs * a boolean "tab-detached" signal: gboolean (* tab_detached) (GtkNotebook *notebook, GtkWidget *child, guint page_num); if the signal handler returns TRUE, the GtkNotebook DnD code will automatically remove the tab from the source notebook and put it in the dest one, if it returns FALSE nothing would happen. This approach would only work with drags within the same application, but would offer a way better API than the current, and handling multiprocess DnD would be really painful anyway. What do you think? Regarding your suggestion about snapping back the tab, I've tried it doing gtk_drag_finish() during _drag_motion()... but doing so leaves the pointer grabbed, I'll try to have a deeper look at the gtkdnd.c code to see if it can be fixed cleanly. Dropping to the desktop still requires other modifications to gtkdnd.c, I think I've identified the code piece Owen talked about in IRC, but fixing this might require another signal hook for knowing that a drop has happened in a dest widget that doesn't accept such targets, please correct me if I'm wrong
Created attachment 58289 [details] tab detaching example, quite hackish
Created attachment 58603 [details] testdetach.c Here is a slightly improved version of the detach example, stealing some code from testgtk to detect the widget under the pointer, and carry child properties over, so that you can detach tabs more than once.
Re #25, I think you are right about dropping on non-accepting targets. GTK+ has support for dropping on the root window (I just fixed it to work again). That is handled in part by gdk, by setting a special protocol, which is then handled in gtk_drag_drop(). Doing something similar for drops on non-accepting targets could be done in two ways: 1) add a signal like GtkWidget::no-drop 2) using another special target like application/x-no-drop in either case, the code in gtk_drag_button_release_cb() will have to be changed to call gtk_drag_drop even if action is 0, and gtk_drag_drop needs to do the right thing if the dest_window is non-NULL but action is 0. I haven't investigated the problems with reattaching yet.
another alternative, maybe more elegant, would be to simply treat nonaccepted drops on rhe desktop background window like root window drops.
Created attachment 58660 [details] [review] treat rejected drops on the background like root drops
With this patch, you should be able to detect drops of notebook tabs on the desktop background, by registering the "application/x-rootwindow-drop" target. You can look at testdnd.c for an example how to do this.
Re #25: I think inter-app dnd is completely irrelevant for this. I think the proposed higher level api is a good idea. I think there should also be a corresponding tab-attached signal. On open question is how you tell the tab_detached handler wether you are going to reattach the tab to another notebook or drop it on the floor (for root window drops)
Created attachment 59195 [details] [review] patch, fourth draft
Issues in the last patch: - GtkNotebook runs out of signal slots - GtkNotebook hasn't any pointer to private data, we should probably replace some private pointer in the GtkNotebook struct with gpointer priv, maybe GList *first_tab is a good candidate - Regarding root window dropping, I've added gtk_notebook_set_window_creation_function(), which specifies a function to create the window that contains the dest notebook (applications might want to create custom ones, with menus, toolbars, etc...), such function has to be responsible of doing window moving/resizing, and creating the notebook. Perhaps this is becoming too awkward for a little win? this is the window creation API: typedef GtkNotebook* (*GtkNotebookWindowCreationFunc) (gint x, gint y, gpointer data); void gtk_notebook_set_window_creation_function (GtkNotebook *notebook, GtkNotebookWindowCreationFunc func, gpointer data, GDestroyNotify destroy);
Created attachment 59196 [details] test sample, a bit more complete and using the new API
Nice, in general. Some comments: - None of the fields in GtkNotebook are explicitly marked as private, thus I would not want to replace any with a private pointer. I don't think this is too terrible. - Regarding exhausting the space in the class, note that signals don't absolutely need to have a slot in the class, unless you want to have a non-NULL class handler. - Would it not enough to have a global hook for window creation, and pass the source notebook to it ? like typedef GtkNoteBook* (*GtkNotebookWindowCreationFunc) (GtkNotebook *source, GtkWidget *page, gint x, gint y, gpointer data); void gtk_nodebook_set_window_creation_hook (GtkNotebookWindowCreationFunc func, gpointer data, GDestroyNotify destroy); Passing the source notebook as a parameter allows you to avoid stuffing the group id in via the userdata. I think this function should be allowed to return NULL, causing the drop to fail. One thing I'm worried about is that making the whole notebook a dnd target will interfere with dnd targets in the content. This needs testing, e.g. does dropping text on an entry in a dnd enabled notebook still work ?
There are some minor todos left in the actual dnd handling. - reattaching was mentioned already (but I think we can handle it in a second step after getting basic dnd support in, if needs be) - when switching from sliding to dnd, the dragged tab jumps back to its original position. It should snap to the closest position near to its current sliding position - when dropping a tab onto a notebook, it jumps to the last position. thats fine when dropping on the content, but when dropping in the tab row, it should go to the position closest to where it was dropped.
Oh, and destroy needs to set pointers to NULL after freeing them, since it may be called more than once (got warnings due to that).
Re #36: - I've removed tab_detached, tab_attached and tab_reordered from GtkNotebookClass declaration, so this still leaves some empty slots. - Good idea with the global hook, the next patch implements it. - As far as I've tested, notebook DnD doesn't interfere with contained widgets DnD, but it feels a little strange to have "holes" where notebook DnD doesn't work (i.e.: inside GtkEntries), maybe we shouldn't allow notebook DnD outside tabs area. Re #37: The only issue left is the first one, as far as I could investigate, gtk_drag_dest_motion() in gtkdnd.c should need a way to know that the drag has been finished to ungrab pointer and so, but couldn't see a way to get such information. Re #38: oops, fixed :)
Created attachment 59414 [details] [review] fifth draft
Created attachment 59415 [details] test sample
Created attachment 59424 [details] [review] sixth try Oops, I saw a case where it would emit spurious "tab-reordered" signals, it's now fixed
Created attachment 59512 [details] test with entries instead of labels, to test text dnd
I think the current patch is basically ready to be committed. Some small things to clean up before committing: - Add "Since: 2.10" to all doc comments for new api. - Add a comment explaining the commented out code block in gtk_notebook_drag_motion () - It should be documented that window_creation_hook may return NULL. One small problem with that is that you emit a tab-detached signal before calling window_creation_hook, so if you get NULL, you probably have to emit tab-attached on the original notebook, since the page does not actually move somewhere else. - The docs for gtk_notebook_set_group_id should probably explain what -1 means: "drop everywhere", or "no dnd". - The setters for group-id, reorderable and detachable code should probably check if the value actually changed before sending notify (although we don't promise this, we normally do it for integer properties).
Oh, one more thing to do before committing this is to write a little mail to gtk-devel explaining the new api, so people get a chance to comment on it. Will you do that, or should I ?
Looking at the tests between the earlier attachment 58603 [details] and the newest attachment 59512 [details], I see that the first moved the tab from old to new notebook itself, while the new one doesn't do that... so I assume that gtknotebook DND code does that for you now with the latest patch? Is there still a way to do this manually? That'll be required to port Epiphany to this (we create special tab label widgets for each tab before inserting it into the notebook).
Matthias: I'm already writing the mail, will handle the other issues tomorrow morning. Christian: In the current code GtkNotebook moves both the content and the tab label widget from one tab to another (assuming they're both in the same group), can't the tab labels be reused this way when moving tabs in ephy?
chpe, you currently have two signals, tab-detached and tab-attached which are emitted before/after the actual transfer happens. Is that not enough for you to modify the tab labels as required ?
Created attachment 59585 [details] [review] another patch The commented code block has been removed, it was merely an sketch of what should do when snapping back the tab to the source notebook, GtkNotebookWindowCreationFunc has been documented, the rest of the issues have been fixed. Besides that, the reorder_tab() function has been slightly improved to avoid unnecessary reordering under some side cases (tabs with alternate packing)
One small thing: treeview header reordering supports Alt-Home and Alt-End to go to either end. We should probably do that here, too.
Also, with the DND keynav change I just submitted, you could theoretically let Alt-Up/Down start a drag (and suitable other arrows for rotated notebooks). Not sure its a good idea, but it should give some keynavigatable way to detach tabs and move them to another notebook. A better ui for that may be a dialog listing the notebooks that you could transfer the tab to, popped up from a tab context menu. I'm not sure if such a dialog should be proved by the notebook, or if each app should do its own. If apps were to do that on their own, they probably need api to list all notebooks with a given group id. If the notebook were to provide it, it needs some way to refer to the notebooks, like a "title" or something. (even cooler, but way more complicated would be Alt-Tab style cycling between all candidate notebooks)
You're right with the alt+[home|end], I'll work on this. Regarding doing detaching keynavigable, with the current API there's no way to know which (nor how many) notebooks are in the same group, that could require something a la GtkSizeGroup. In my humble opinion, most consumers of this new API (I'm mostly thinking of ephy, gnome-terminal and gedit) will only need one group for their tabs, and will know much better than gtk itself the candidates for DnD (at least with the current API), so maybe a GtkNotebookGroup class is way too much for their needs? can they manage this task themselves better than gtk could? Another smallish issue is that Alt+[Up|Down] is already used in notebooks with tabs to the right or left, so alternating too much the keybindings could be confusing... But that's just an opinion, I'll be glad to work on it if you still feel that it's worth the effort :).
Yes, I would not worry about the group thing for now. And the Alt-Up/Down is something that we can try out after landing the dnd support. Its a 5 line patch on top of your work, so it'll be easy to add afterwards. The only thing that currently holds off committing this is getting a comment from chpe on the sufficience of this for epiphany.
Created attachment 59655 [details] [review] updated patch It adds support for Alt+[Home|End] keybindings
I've looked over epiphany's notebook code again to see how this will fit with it. Epiphany needs: - a wrapper around gtk_notebook_insert_page, that * builds a custom tab label widget for the tab, and * emits a tab-added signal after the tab has been inserted, and - a wrapper around gtk_notebook_remove_page which emits a tab-removed signal after the tab has been removed. The tab labels that we build are not reusable in a different notebook, because - older epiphany versions connected a signal to the notebook with tab as user_data (current version doesn't anymore, but I think it should still be supported) - sets tooltip on the tab label in a GtkTooltips object that belongs to the notebook. I've looked at the patch here, and have a few problems with it: - the tab_attached signal works like my tab-added signal, but is only emitted on newly attached-by-DND tabs. - same for the tab_detached signal wrt. my tab-removed signal - the comment for tab_detached says + * A handler for the ::tab-detached signal should do any aditional + * cleanup needed for detaching the tab and then return %FALSE to + * allow tab detaching, or return %TRUE to deny it. so what happens if one handler does that 'cleanup', and then a later handler (added from an extension in epiphany, for example) decides to veto the detach? - the notebook does the moving from one notebook to another one on DND itself, reusing the tab label, breaking everything epiphany needs to do here (tab-removed signal AFTER removal, and using a new tab label). - the tab_reorderer signal is fine So how can we fix this? - we could make tab-attached into tab-added - split tab-detached in tab-detach-request (return TRUE to deny) and tab-removed after the removal has been done For building the tab label I could see two ways: - make gtk_notebook_insert/remove_page call class vfuncs where you could build the tab label and ignore the passed-in widget or - add a GtkWidget * (*GtkNotebookBuildTabLabel)(GtkNotebook*nb, GtkWidget*tab, gpointer userdata) function with gtk_notebook_set_tab_label_creation_hook that builds a tab label for a given tab in the notebook, and use this function when DND moves the tab to another notebook, falling back to the existing label in the origin notebook when this func isn't set?
I think these are good ideas - replace ::tab-attached and ::tab-detached by ::tab-added which gets called after any page addition, and ::tab-removed which gets called after any page removal - add vfuncs for insert/remove, with default implementations that add/remove the page and emit ::tab-added/::tab-removed. - is a separate ::tab-detach signal (which gets emitted before detaching and can veto it) actually necessary ? Is there any use case for dynamically determining detachability, instead of just setting the detachable child property ?
I don't think the ::tab-detach signal is necessary. Epiphany has it to create the new window + move the tab, but this patch deals with that differently, which looks fine to me (window creation func). The only real reason to have a veto-able detach signal would be to check the target notebook, but the signal doesn't carry that, and the group ID thing works just as well, I think.
Created attachment 59969 [details] [review] updated patch - replaces ::tab-detached and ::tab-attached with ::tab-added and ::tab-removed - implements insert_page() vfunc, implementation (gtk_notebook_real_insert_page ()) mostly comes from the old gtk_notebook_insert_page_menu() code - instead of implementing remove_page() vfunc, I've made gtk_notebook_remove_page to use gtk_container_remove(), which actually removes the page and emits ::tab-removed
Forgot to mention that tabs detaching is no longer vetoable, and that a GktNotebookWindowCreationFunc handler returning NULL will no longer emit any ::tab-attached/tab-detached signal equivalents
Looks very good to me. I think we should get this in cvs now, so that ephy and others can easier try it out, and work on any remaining problems in cvs. Very small nitpick: if (class->insert_page) return (class->insert_page) (notebook, child, tab_label, menu_label, position); the if is pointless if you know that there is an implementation (since GtkNotebook has one itself)
2006-02-23 Matthias Clasen <mclasen@redhat.com> * gtk/gtknotebook.h: Add a reorder_tab keynav signal and an insert_page vfunc to GtkNotebook. * gtk/gtk.symbols: * gtk/gtknotebook.c: Support notebook DND. New API includes gtk_notebook_set_window_creation_hook, gtk_notebook_[gs]et_group_id, gtk_notebook_[gs]et_tab_reorderable, gtk_notebook_[gs]et_tab_detachable (#73240, Carlos Garnacho)
I have a need to manually decide whether a tab should be accepted. Is this possible with the current API? Even if I connect to drag-drop and drag-data-received and try to stop them with drag_context.finish, the tab gets moved.