GNOME Bugzilla – Bug 363848
Can't select a different bookmark topic by moving the mouse
Last modified: 2007-04-02 20:36:42 UTC
In most other browsers, once the user has selected a bookmark topic/folder by clicking on it in the toolbar, they're able to move the mouse to a different topic and have its menu pop up. Epiphany doesn't support this, the user must click on the new topic to select it (annoying).
Created attachment 75121 [details] [review] Select topics by moving the mouse Attached patch, but I don't know how GNOME-ish it is. Might need cleanup.
Thanks for the patch. First, I'm not sure this is really expected behaviour; a toolbar is not a menubar after all. Still here's a comment on the patch: + static GtkWidget* activated_button = NULL; + static EphyTopicAction* activated_action = NULL; [...] + activated_button = button; + activated_action = action; No, no, no. :) Using static variables is just wrong.
I can't find any other gnome applications that use a toolbar with drop-down menus, but the expected behaviour in web browsers is indeed to treat bookmark tool bars as menu bars. As for the patch, yes it is very ugly; the activated_* variables should be attached to the bookmark list, though I have no idea how to do that.
I agree with John, it would be nice if topic dropdowns would open automatically if you move the mouse cursor to it from an open neighbouring topic. Even if we won't do this, it's wrong that opening a different topic requires two mouse clicks, one to close the open topic and one to open the second one.
Created attachment 84005 [details] [review] Select topics by moving the mouse Now with 100% less 'static'
harves, could you take a look to see if this is correct? It looks ok to me, apart from the codestyle issues (commented on IRC about these).
Created attachment 84103 [details] [review] Select topics by moving the mouse, r3 Same code as the last patch, with correct coding style
Looks good to commit as far as I'm concerned. There are some minor things that could be changed, like finding a way to avoid recording the action; just recording the button should be sufficient, and button_toggle_cb might be the best place to call erase_popup, but I'm not sure, and none of it is necessary. Nice work. :)
Nice effect :) + g_object_set_data (G_OBJECT (window), + "active-topic-action-button", + button); + g_object_set_data (G_OBJECT (window), + "active-topic-action", + action); [...] + g_object_set_data (G_OBJECT (window), + "active-topic-action-button", NULL); + g_object_set_data (G_OBJECT (window), + "active-topic-action", NULL); [...] It's enough to store the button, since you can get the action with gtk_widget_get_action from it. I'm a bit uncomfortable with unsetting the data on button_toggled_cb, since there are instances where the menu is deactivated but the button not unpressed, e.g. when we get a grab shadowing or another tab tries to steal the focus. (That's a bug, but I'd rather be safe here).
Actually, calling gtk_widget_get_action on the active button always returns NULL. Is the action is supposed to be recorded along with the button? If so, I could open another bug. By unsetting the data in button_toggled_cb, I hoped to avoid any situation where active-topic-action is non-NULL but no menu exists. I figure it's better to have a button stuck down, which is current behaviour in the face of such an error.
I realised that the get_action returns NULL because the toolbar items aren't created by gtkuimanager. Yes, but if the button is stuck down there won't have been a toggled signal (else it would be 'up') and thus the old action is still set as data on the window. If the action is now destroyed (e.g. by removing the topic), there might be a crash...
Created attachment 85418 [details] [review] Select topics by moving the mouse, r4 This patch sets the data "gtk-action" on toolbar buttons, removing the need to store the action separately. It also moves unsetting "active-topic-action-button" into button_deactivate_cb, which is called upon the menu popup's "deactivate" signal.
Looks fine to me now.
Thanks John!. Committed to trunk, rev 6981. 2007-04-02 Diego Escalante Urrelo <diegoe@gnome.org> * src/bookmarks/ephy-topic-action.c: Allow the user to select a different bookmark topic on the toolbar by just moving the mouse. Old behaviour forced the user to click each topic button to activate the menu, now only the first click is required. Bug #363848. Patch by John Millikin.