GNOME Bugzilla – Bug 161613
New menu layout
Last modified: 2005-01-09 21:00:47 UTC
Here are screenshots and a patch for a new menu layout (most of the design ideas are jdub's). The patch could bear with more love, and I'll work a bit more on the code once we agree on the design. I'd like to get this in HEAD ASAP (2.9.3 would be great, but 2.9.4 is a more likely target), but I prefer to collect some comments before I commit. Note that this only changes the menu bar. I'll update the main menu if this gets in.
Created attachment 34962 [details] Screenshot of the menu bar
Created attachment 34963 [details] Screenshot of the first menu
Created attachment 34964 [details] Screenshot of the second menu
Created attachment 34965 [details] Screenshot of the third menu
Created attachment 34966 [details] [review] gnome-panel patch
Created attachment 34967 [details] [review] gnome-menus patch You'll also need to patch gnome-menus if you want to test the code. Here's a quick patch.
Some random notes I forgot: + we'll probably need to add a desktop file for gnome-about (it'll make things easier for the panel) + the second known bug (see the comment in the panel-menu-bar.c patch) is fixed :-) + "Connect to Server..." launches the same dialog as the one you can see in nautilus right now + the items in the Places menu are: Home Desktop [GTK bookmarks] --- Computer [mounted volumes] --- Network [connecter servers] Connect to Server... --- Search files Recent documents Of course, the menu is updated when a new volume is mounted or when you connect to a new server. + tooltips labels could be enhanced + no one has been able to find a better name for the System menu + implementation detail for Mark: I'm trying to move from action buttons to desktop files when it's possible (e.g. gnome-search-tool)
I would: 1. Remove "Take screenshot", since it's an applet anyway, and people don't generally need it. 2. Add a line between "Lock screen" and "Log out", because clicking the one when you want the other could be quite annoying. Other than that, I'm not entirely sure 3 menus will be a good thing at all.
Good stuff, Vincent. Hopefully the appropriate people will give you feedback on the layout and terminology etc. :-) gnome-panel patch: - I'd be a lot happier if we could just add --connect-server to nautilus and use that rather than copying and pasting the code. I get the feeling that this feature still needs work in Nautilus and we don't want to try and track those changes ... - typo: + desktop_path = g_strconcat (path, ".deskstop", NULL); - in panel_menu_bar_append_from_desktop() you should use panel_lookup_in_data_dirs() - also in panel_menu_bar_append_from_desktop() I'd use the GKeyFile API - GnomeDesktopItem is only really useful for launching stuff these days - for readability I'd rewrite append_volumes() as: { GList *volumes, *l; volumes = gnome_vfs_volume_monitor_get_mounted_volumes (volume_monitor); for (l = volumes; l; l = l->next) { GnomeVFSVolume *volume = l->data; char *icon; char *display_name; char *activation_uri; if (!gnome_vfs_volume_is_user_visible (volume) || !gnome_vfs_volume_is_mounted (volume)) continue; switch (gnome_vfs_volume_get_volume_type (volume)) { case GNOME_VFS_VOLUME_TYPE_CONNECTED_SERVER: if (!connected_volumes) goto next_volume; break; default: if (connected_volumes) goto next_volume; break; } icon = gnome_vfs_volume_get_icon (volume); display_name = gnome_vfs_volume_get_display_name (volume); activation_uri = gnome_vfs_volume_get_activation_uri (volume); panel_menu_bar_append_place_item (menubar, icon, display_name, display_name, //FIXME tooltip menu, TRUE, G_CALLBACK (activate_uri), activation_uri); g_free (icon); g_free (display_name); g_free (activation_uri); next_volume: gnome_vfs_volume_unref (volume); } g_list_free (mounted_volumes); } (the "uri" arg to append_place() should be a const char *) - won't + gconf_client_remove_dir (panel_gconf_get_client (), + DESKTOP_IS_HOME_DIR_DIR, + NULL); screw us over if we've two menu bars and you remove one? gnome-menus patch: - It seems like we don't want to have any Core items in the menu, so rather than a bunch of <Exclude>s, why not just remove the toplevel <Include> ? - There seems to be a duplicate System Configuration <Menu> in settings.menu - I'm not sure "Desktop Preferences" is going to jive with the docs people - I think according to the GDSG "desktop" refers literally to desktop background and icons. Maybe "Personal Preferences" or something might communicate the difference with "System Configuration" we are looking for.
The only comment I have on the screenshots is to use title capitalization for lock screen in the third screenshot. That is, change "Lock screen" to "Lock Screen". We could avoid getting into the whole "desktop" debate again but just renaming that menu "Preferences", which I think would work well.
Vidar: 1. "Take screenshot" is not an applet, but an action button. There is no difference for the user, but I'm not sure we'll keep all the action buttons forever. 2. There would be a lot of separators, then. But I totally agree with the reasons you give... Eugene: Thanks, I'll update labels. Mark: > gnome-panel patch: > > - I'd be a lot happier if we could just add --connect-server to nautilus > and use that rather than copying and pasting the code. I get the > feeling that this feature still needs work in Nautilus and we don't > want to try and track those changes ... I was more or less thinking of removing this from nautilus. This would need discussion, of course. > > - typo: > > + desktop_path = g_strconcat (path, ".deskstop", NULL); > > - in panel_menu_bar_append_from_desktop() you should use > panel_lookup_in_data_dirs() I know. I did not update before making the patch :-) > - also in panel_menu_bar_append_from_desktop() I'd use the GKeyFile API > - GnomeDesktopItem is only really useful for launching stuff these days Ok. > - for readability I'd rewrite append_volumes() as: > > { > GList *volumes, *l; > > volumes = gnome_vfs_volume_monitor_get_mounted_volumes (volume_monitor); > > for (l = volumes; l; l = l->next) { > GnomeVFSVolume *volume = l->data; > char *icon; > char *display_name; > char *activation_uri; > > if (!gnome_vfs_volume_is_user_visible (volume) || > !gnome_vfs_volume_is_mounted (volume)) > continue; > > switch (gnome_vfs_volume_get_volume_type (volume)) { > case GNOME_VFS_VOLUME_TYPE_CONNECTED_SERVER: > if (!connected_volumes) > goto next_volume; > break; > > default: > if (connected_volumes) > goto next_volume; > break; > } > > icon = gnome_vfs_volume_get_icon (volume); > display_name = gnome_vfs_volume_get_display_name (volume); > activation_uri = gnome_vfs_volume_get_activation_uri (volume); > > panel_menu_bar_append_place_item (menubar, > icon, > display_name, > display_name, //FIXME tooltip > menu, TRUE, > G_CALLBACK (activate_uri), > activation_uri); > > g_free (icon); > g_free (display_name); > g_free (activation_uri); > > next_volume: > gnome_vfs_volume_unref (volume); > } > > g_list_free (mounted_volumes); > } Can I take this as an "authorization" to use goto whenever I want? It would be useful in some other places... > (the "uri" arg to append_place() should be a const char *) Indeed. > - won't > > + gconf_client_remove_dir (panel_gconf_get_client (), > + DESKTOP_IS_HOME_DIR_DIR, > + NULL); > > screw us over if we've two menu bars and you remove one? From the gconf doc: You can also add "/foo" multiple times; if you add a directory multiple times, it will not be removed until you call gconf_client_remove_dir() an equal number of times. > gnome-menus patch: > > - It seems like we don't want to have any Core items in the menu, so > rather than a bunch of <Exclude>s, why not just remove the toplevel > <Include> ? Ok. > > - There seems to be a duplicate System Configuration <Menu> in > settings.menu I pasted this from applications.menu. I was not sure why there were two System Configuration <Menu>, so I kept them. > - I'm not sure "Desktop Preferences" is going to jive with the docs > people - I think according to the GDSG "desktop" refers literally to > desktop background and icons. Maybe "Personal Preferences" or something > might communicate the difference with "System Configuration" we are > looking for. See Eugene's comment. I'll update the patches asap.
> I was more or less thinking of removing this from nautilus. This would need > discussion, of course. Well, I'd prefer it to live in Nautilus, but as long as it only lives in only one place I don't really mind. > Can I take this as an "authorization" to use goto whenever I want? It would be > useful in some other places... I couldn't make up my mind whether to have a goto (which I try to except for freeing stuff at the end of the function, but I thought this case was obvious and very readible. But, notice the code above has a bug - the first "continue" should be a "goto next_volume". So, in that case I think I'd prefer to just have g_slist_foreach (volumes, (GFunc) gnome_vfs_volume_unref); after the loop. > > - There seems to be a duplicate System Configuration <Menu> in > > settings.menu > > I pasted this from applications.menu. I was not sure why there were two System > Configuration <Menu>, so I kept them. Ah, okay. It looks very wrong - feel free to fix. > From the gconf doc: > You can also add "/foo" multiple times; if you add a directory multiple times, > it will not be removed until you call gconf_client_remove_dir() an equal > number of times. Indeed, that'll teach me for glancing at the code and misreading it instead of using the API docs :-)
I like the looks of this and I'm really glad you're working on it, here are my suggestions Not having Run Application as a top level [Application] menu item. The Applications menu seems like a logical place for it, but being a top level item make it look like something the average user would need to use GNOME consistantly. I'd like to see something like the quickstart menus here (bug 130733) Let me get comments on [Places] later. I know this has been discussed before so I think all further discussion should take place on bug 89874 so I've put my comments in there to why I think we should use Desktop and not System. For the rest of the menu I'd like to see Desktop Preferences renamed to "Preferences" and System Configuration to "Administration" thus Desktop -> Preferences -> Administration Otherwise I think you've done a great job at combining the best parts of what other requests have been made. The Help menu and and About menu and Preferences being moved.
Bryan: thanks for the bug triaging! I'll add a comment on the quickstart menus bug later. Right now, I'm wondering: if Run Application is not a top level menu item, where should it live?
Sorry for the spam. Confirming some bugs. Filter on "VINCENT CONFIRMS" to ignore.
Vincent, off the top of my head I'd probably drop it back in Desktop/System.
Hi, I think that this layout is really nice, it specifically gives a good solution to the "desktop preferences are not applications" problem and to keep separated system tools (i.e.: gnome system tools) from other "system tools" which don't need root password (i.e.: file-roller, gconf-editor, ...) Another big problem (at least for me) has been the desktop preferences clutter (right now gnome CVS has about 20 items there, some of them in submenus), but it has become much more than a layout problem, so I don't expect to get it fixed with these patches... So I'd vote for this getting included in 2.10 :)
I think we should definitely try to get the patches in to 2.10, so don't let this comment delay it :-) "Recent Documents" is basically useless buried as it is. We need to decide if the idea of a desktop-wide recent documents menu makes sense. If it is useful, we should promote it to a top-level item. If it is not, we should drop it.
Created attachment 35518 [details] [review] Updated panel patch I've modified the panel patch. Thanks to all the comments. I need to clean some small things before I commit it. It should be in 2.9.4. I removed the "Connect to Server" menu item for now since I don't want to copy the code from nautilus. I'll talk with Alex about that.
Created attachment 35519 [details] [review] Updated gnome-menus patch
Maybe I missed this one, but where is the "Search for Files..." menu item? Thanks. ;-)
It's in the 2nd menu screenshot. It's only in french "Rechercher des Fichiers" ;-)
yeah, might be nice if someone redid the screenshots in english with the latest patches... I'd like the novell group to take a look at them, and don't have a HEAD build ATM.
I am liking this. I'll try to give a more in-depth reply soon, but so far this looks very good and clean. Go go go! :)
Comment on attachment 35518 [details] [review] Updated panel patch I tried applying this patch to CVS HEAD and these two files had problems applying cleaning. panel-action-button.c ^^ Just a Lock [S|c]reen conflict. I guess the string change has already been committed. panel-menu-bar.c: Large changes didn't apply.
This is now in HEAD. Please open new bugs for specific problems. Thanks