GNOME Bugzilla – Bug 130657
Menu stripe patch
Last modified: 2020-11-07 12:16:22 UTC
This is Mark's rewritten XD2 menu stripe patch for JDS. It's good to go in, if it makes sense usability people.
Created attachment 22999 [details] [review] menu stripe patch
Upgrading the priority level to high. (I am actually Gman's indentured servant, not slave.)
*** Bug 76873 has been marked as a duplicate of this bug. ***
The question is: do we want to have this? usability people: any advice on this?
(Trying to be unbiased here :-) Its a strange situation - in general we're trying to get away from "branding" GNOME, so this patch isn't something we want on in GNOME by default ... but because at least two GNOME distributors use the feature and seeing as its often requested, I'm inclined to think we should include it. Not fully convinced, though - definetly want other people's opinion.
(note that if we'll include it, I'm sure we'll have lots of people asking for a gconf key to disable it)
Yeah, I'm saying if we were to include it, it would be optional and off by default (yes, that does sound stupid)
Can't say I really have much of an opinion on it either way, except that vertical text in a GUI generally sucks so I doubt if anybody ever reads it anyway :)
I don't understand why the menu stripe should be ruled out on branding reasons. Yes, branding went way overboard in Gnome 1.x; but removing all branding would mean doing away with (or changing) the splash screen and removing the "About GNOME" option from the panel. I don't see how this is any different than those two things. If it is different, could someone explain it to me? If however, the menu stripe is ruled out because vertical text looks bad, or it makes the menu harder to use for general users or something like that, then I understand--but I hope that isn't the case because I personally really like the stripe. :-)
Created attachment 29607 [details] [review] Updated patch with gconf key Updated patch to current cvs and with a boolean gconf key default to false.
David: could you attach a patch with "cvs diff -upN" so we can see the new files? Thanks
Created attachment 30786 [details] [review] patch with new files in There was no changes in the new files from the original patch, IIRC. This is a single patch with the two new files in.
The gconf key should probably not be global, but per menu.
+ retval = panel_menu_new (FALSE); Could that be made into a compiletime option, or isn't that needed ? + if (!menu) { menu = create_panel_root_menu (panel_widget); + panel_menu_set_stripe_enabled (PANEL_MENU (menu), TRUE); + } Why does it send in TRUE here without checking ? (I don't know the code around here, so excuse me if it already does) + * panel-menu.c: A GtkMenu with an ugly stripe down the side So you actually think it's ugly yourself ? ;) +#define PANEL_MENU_DEFAULT_IMAGE "gnome-panel-menu-stripe" +#define PANEL_MENU_DEFAULT_GRADIENT_TOP { 0, 0xffff, 0xffff, 0xffff } +#define PANEL_MENU_DEFAULT_GRADIENT_BOTTOM { 0, 0x0000, 0x0000, 0xffff } What about wrapping those in #ifndef, so they can be overrided ? Or are these just defaults ? + goto out_no_name; + goto out_no_path; Generally, I don't like goto's, but I don't know the convention in gnome-panel. I'm also very impressed by the total lack of comments in the patch. In any case, I'm just voicing my .2 NOK, feel free to ignore ;)
Created attachment 33773 [details] [review] menu stripe patch for gnome-panel-2.8.1 Updated patch for gnome-panel-2.8.1
This can't go in on 2.8.x since it's feature frozen and this patch adds features and strings etc. Could this be rebased against the new menu implementation or is it obsolete now?
Comment on attachment 33773 [details] [review] menu stripe patch for gnome-panel-2.8.1 A few things needs to happen here: 1) Decide whether we want this or not - I'm leaning towards no now. Adding something we don't actually want to use seems crack 2) The patch needs to be updated for gnome-panel HEAD 3) Need to investigate if we could do this in a less hacky way. Does the table-based GtkMenu stuff help us at all?
1) I agree that GNOME doesn't need this, but I can see how lots of consumers would like to be able to brand their distros with this patch. Having it in gnome-panel CVS would ease their work, as they wouldn't have to apply it themselves for every release.
Mark: as you said when you worked for someone else ;) there are quite a few folks who want to brand gnome this way- it's not just sun and novell, but also other deploying groups like linex (as I understand it.) Certainly, 'how do we brand GNOME' has been a regular request from a variety of groups. So I'd lean towards 'please include' as long as it won't become a gigantic support headache, which is clearly your call.
> Does the table-based GtkMenu stuff help us at all? Where can I find some example code for this?
I am sympathetic to the trouble this causes developers. At the same time, the "menu stripe" seems wrong. Not terribly egregiously wrong mind you, just a little off. Should we make it easier for distributors to perpetrate the mild evil they're going to do anyway? Dunno.
Created attachment 38880 [details] [review] gnome-panel-2.10.0-2-MenuStripe.patch Quick 'n' dirty update of the patch against gnome-panel-2.10.0. It compiles, but doesn't work. :-]
Someone should really have a look at link's patch, fix it and get it into CVS HEAD... if only to finally close this bug! While I don't see myself using this, I can't make out the problem if this was ifdef'd with a nice '--enable-menustrip' configure oftion which defaults to no.
Created attachment 46787 [details] [review] Updated patch This patch works and builds against 2.10.1
Ooops, hadn't realized that Link's patch had added some gconf code - should be trivial to add to the above though.
Glynn: do you (or someone else at Sun) have sometime to look at Mark's idea (investigate if table-based GtkMenu would work here)?
I'm pretty swamped in OpenSolaris land, but I've passed this onto the team to see if there's anyone keen on picking this up.
Ping.
Anyone have a working patch for 2.16.0?
Created attachment 72437 [details] [review] Test patch This patch works somewhat,it also applies to System menu (not desirable) and make needs to be run from gnome-panel src directory.
FWIW, in the latest versions of JDS, we've decided to drop this patch - though that information isn't a reason to close this bug, it felt right to inform others ;)
Created attachment 72501 [details] [review] Updated patch, fixes places meny Updated patch, now stripe looks correct on Application/Places/System menu.
(In reply to comment #31) > FWIW, in the latest versions of JDS, we've decided to drop this patch - though > that information isn't a reason to close this bug, it felt right to inform > others ;) > Well I think it's still important for Branding. It still would be a nice branding feature, if somebody would add it to the tree.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old feature requests in Bugzilla which have not seen updates for many years. If you still use gnome-panel and if you are still requesting this feature in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/gnome-panel/-/issues/ Thank you for reporting this issue and we are sorry it could not be implemented.