GNOME Bugzilla – Bug 667619
Port to GtkApplication and GtkApplicationWindow
Last modified: 2012-05-16 12:51:22 UTC
This is needed for the app menu integration (bug #656634)
Cassidy had existing WIP: http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=gtk-app-667619
I've rebased cassidy's work and then extended it. I've changed the look and feel of Empathy to remove the menubar from the roster all together: http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=gtk-app-667619 TODO: - add accels to the menu: I'm not sure how to do this - port other windows to GtkAppWindow - add accels for some features now in Preferences, most specifically show hidden? Even with these TODOs it would be good to review and merge this to uncover any bees from the work.
Created attachment 213846 [details] looks like this
Didn't review the code yet, so there are just comments based on the screenshot: - Nice! :) - I'd remove the 'Edit ...' so have just 'Accounts' and 'Blocked Contacts' - What's the point of the 'Close' menu? We can just close the window using the WM close button. - We shouldn't duplicate entries which are in the global menu ( help / debug / about / preferences etc); Web doesn't. - I think we should use the same icon as Web for this button.
(In reply to comment #4) > - I'd remove the 'Edit ...' so have just 'Accounts' and 'Blocked Contacts' Do you think? > - What's the point of the 'Close' menu? We can just close the window using the > WM close button. Okay. > - We shouldn't duplicate entries which are in the global menu ( help / debug > / about / preferences etc); Web doesn't. The button and the global menu are actually more or less identical. I'm doing this on purpose because I wanted to disable the menubar on platforms without the global menu (e.g. XFCE) and I wanted it to work for people with focus follows mouse. I don't think it's a bad thing. > - I think we should use the same icon as Web for this button. Any idea what it's called?
Using Shell 3.4, I don't get any app menu, not even the default one with 'Exit', nothing. I got this when opening the preferences dialog: (empathy:3571): Gtk-WARNING **: Unknown property: GtkGrid.n-columns /* FIXME: display accelerators in menu */ They are displayed here. http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=gtk-app-667619&id=5430d8b70bb24850c0ba5c5f67b1962adb598f0c Don't we want to use a subset of this menu? http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=gtk-app-667619&id=f1b6abb6f40cb4d4ecde6cae6091a7e5ad9df7fe Do we have any UI to tweak this gconf key? If not, I think it should default to True at least. Maybe that's something we could add to gnome-tweak-tools? In the preferences dialog, the padding between "Contact List" and the first item seems to be lower than in the other sections.
(In reply to comment #5) > (In reply to comment #4) > > > - I'd remove the 'Edit ...' so have just 'Accounts' and 'Blocked Contacts' > > Do you think? I think so, the 'Edit' doesn't really buy us anything. > > - We shouldn't duplicate entries which are in the global menu ( help / debug > > / about / preferences etc); Web doesn't. > > The button and the global menu are actually more or less identical. I'm doing > this on purpose because I wanted to disable the menubar on platforms without > the global menu (e.g. XFCE) and I wanted it to work for people with focus > follows mouse. I don't think it's a bad thing. what do you mean by 'disable the menubar'? If Empathy is running on a system not supporting the app menu, Gtk will do the fallback itself and include a menubar with a 'Empathy' menu containing the app menu. > > - I think we should use the same icon as Web for this button. > > Any idea what it's called? Looks like that's emblem-system-symbolic
(In reply to comment #6) > Don't we want to use a subset of this menu? Could do. Although by having the whole menu it makes all the accelerators work for free. This is fixable, but effort~ > Do we have any UI to tweak this gconf key? If not, I think it should default to > True at least. Maybe that's something we could add to gnome-tweak-tools? No, because I forgot. (In reply to comment #7) > what do you mean by 'disable the menubar'? > If Empathy is running on a system not supporting the app menu, Gtk will do the > fallback itself and include a menubar with a 'Empathy' menu containing the app > menu. It won't because I intentionally disabled it. That can be reverted, but IMO looks a little naff on things not supporting the global app menu.
So, here is I think what we should do for the first iteration of this work regarding the menus: - Remove the close menu entry - Only include the items listed in https://bugzilla.gnome.org/show_bug.cgi?id=656634#c3 in the app menu - Remove these items from the button-menu - Allow Gtk to fallback when running in a desktop not supporting the app menu About the preferences options: - Display the balance in the roster by default. - Don't add it to the preferences dialog. We'll keep it as a 'secret' gsetting for now and will consider adding an option to gnome-tweak-tools if we need to - Remove the roster view options from the preferences dialog as well (but not 'show offline contacts') as they will probably go away with the new roster design anyway.
The app menu format I suggested in bug 656634 was meant to avoid the need to restructure or move the existing menubars, so I'm not sure why it is being used as a justification for this bug... Anyway, looking at http://bugzilla-attachments.gnome.org/attachment.cgi?id=213846, the problem you have is that all those entries conceptually fit into the app menu. How is a user going to understand which menu to look in? The answer might appear to move everything to the app menu. That would be fine, except you have too many menu items for that. My best attempt to fit everything in looks like this: New Conversation... New Call... --- Contacts > --- Rooms > --- Accounts Preferences --- Help About Empathy Quit Where Contacts contains Add, Search, Map and Blocked. (I've omitted Debug. I don't think that belongs in a user-facing application, plus it's YAMI (Yet Another Menu Item).) The Rooms menu could be pretty ugly here, but it could be worth a try.
Ugh, I forget File Transfers and Previous Conversations, didn't I? Yea, that's never going to work.
(In reply to comment #10) > The app menu format I suggested in bug 656634 was meant to avoid the need to > restructure or move the existing menubars, so I'm not sure why it is being used > as a justification for this bug... So you'd avocate to keep the existing menu bar (modulo the simplification we made in this brack like removing the different view options)? But then, what's the point of havint his app menu if it doesn't simplify the main window? > Anyway, looking at > http://bugzilla-attachments.gnome.org/attachment.cgi?id=213846, the problem you > have is that all those entries conceptually fit into the app menu. How is a > user going to understand which menu to look in? That's kinda the same problem if we keep the existing menu. Why would user uses the app menu instead of window one if it's not sure if he will find the option there or not? > The answer might appear to move everything to the app menu. That would be fine, > except you have too many menu items for that. My best attempt to fit everything > in looks like this: > > New Conversation... > New Call... > --- > Contacts > > --- > Rooms > > --- > Accounts > Preferences > --- > Help > About Empathy > Quit > > Where Contacts contains Add, Search, Map and Blocked. Grouping things in 'Contacts' is actually a very good idea. (I've omitted Debug. I > don't think that belongs in a user-facing application, plus it's YAMI (Yet > Another Menu Item).) Debug has been proofed really really useful when debugging issues with user. I'm fine removing it from the menu but we need a way to make it very easy for the user to start it. (In reply to comment #11) > Ugh, I forget File Transfers and Previous Conversations, didn't I? Yea, that's > never going to work. Maybe removing 'File Transfers' isn't that bad (it's automatically displayed when a FT is started) and ideally it should be in the log viewer as well. 'Previous Conversations' is a must have unfortunatelly.
So, after discussing this on IRC, here are the items we are going to put in the app menu so we won't need the 'button menu': New Conversation New Call Contacts Accounts Previous Conversations File Transfers Rooms Help About Empathy Preferences Quit And we are going to consider replacing the 'Rooms' menu by a toggle in the main window but I don't think we should block on it.
http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=gtk-app-667619-rework Reworked version. Dropped some of the now-useless commits but did not spend heaps of time crushing squashing commits back together from the direction change, as it was going to be a PITA. Wasn't sure about 8582dfb163 but we can keep it or not as people want. Random aside we need to tweak the account balance roster to make it look less crappy. Sort of tempted to move it to the bottom of the roster as an 'inline-toolbar'. Is there a symbolic icon to replace 'Top Up'.
(In reply to comment #6) > I got this when opening the preferences dialog: > (empathy:3571): Gtk-WARNING **: Unknown property: GtkGrid.n-columns I'm still experiencing this. We discussed removing some 'view' options from the preferences dialog. I think I'd remove at least 'Contact size' (it doesn't look great in the preferences dialog and is by far the less useful option). What do you think? (In reply to comment #14) > Reworked version. Dropped some of the now-useless commits but did not spend > heaps of time crushing squashing commits back together from the direction > change, as it was going to be a PITA. Sure, no no problem. > Wasn't sure about 8582dfb163 but we can keep it or not as people want. I think I'd drop it. It looks better without it. > Random aside we need to tweak the account balance roster to make it look less > crappy. Sort of tempted to move it to the bottom of the roster as an > 'inline-toolbar'. Is there a symbolic icon to replace 'Top Up'. Yeah sounds like a good idea to me (can be done in another bug).
Looks good! Thanks a lot for your work on this.
*** Bug 656634 has been marked as a duplicate of this bug. ***
Oh I just noticed that Ctrl+H doesn't display/hide offline contacts any more. Could you please restore that?
Merged current branch to master. Need to fix accelerator regressions: Ctrl-H (hidden), Ctrl-J (join room) and Ctrl-Q (quit).
Ctrl-h fix: http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=misc
(In reply to comment #20) > Ctrl-h fix: > > http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=misc ++
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.