After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 621356 - Add subtitles to popup menus
Add subtitles to popup menus
Status: RESOLVED INVALID
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Colin Walters
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-06-12 09:16 UTC by Giovanni Campagna
Modified: 2010-06-25 18:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add support for subtitles in popup menus (1.96 KB, patch)
2010-06-12 09:16 UTC, Giovanni Campagna
none Details | Review
Corrected patch (1.97 KB, patch)
2010-06-12 09:26 UTC, Giovanni Campagna
none Details | Review
Another possibility (2.08 KB, patch)
2010-06-12 09:51 UTC, Giovanni Campagna
none Details | Review

Description Giovanni Campagna 2010-06-12 09:16:15 UTC
(This is part of bug 621017)

You should add a generic API for creating non reactive menu items that are clearly distinguished from the others, like separators with text in them.
What I am thinking about are "Wired networks" / "Wireless networks" in nm-applet or "Devices" in gnome-bluetooth.
Comment 1 Giovanni Campagna 2010-06-12 09:16:56 UTC
Created attachment 163457 [details] [review]
Add support for subtitles in popup menus

Creates class PopupSubtitleMenuItem, which is a non-reactive menu
item containg a label with class 'popup-subtitle-menu-item', plus
adds convenience methods to PopupMenu.
Comment 2 Giovanni Campagna 2010-06-12 09:26:40 UTC
Created attachment 163458 [details] [review]
Corrected patch

sorry, a comma was missing
Comment 3 Giovanni Campagna 2010-06-12 09:51:31 UTC
Created attachment 163459 [details] [review]
Another possibility

Adds a second argument to the constructor, to override the default styling. Still, the method shorthand does not provide it, to accommodate the most common case.
Comment 4 Dan Winship 2010-06-25 17:47:29 UTC
This doesn't correspond to the style of anything in the mockups; in your NM patches you use it for, eg, "Wired networks". But in the mockups, that line is dimmed, not bold. And other items with the same style (eg, "Wi-Fi") have switches anyway, so there's not a one-to-one link between that text style and whether a menu item is selectable or not.

It may turn out to be useful to add additional PopupMenuItem arguments to easily override things like reactive and style_class; we need to go through the mockups and figure out the set of features that are needed to implement all of the things there.
Comment 5 Giovanni Campagna 2010-06-25 17:51:47 UTC
Except that they're not "easily override", as the label is not a public property, so you cannot (or should not) set the style_class on it.

Secondly, my patch is focused on adding subtitles. Styling them is a matter for theme designers (I added bold just to make it clear they're different from the rest).
Comment 6 Dan Winship 2010-06-25 18:12:16 UTC
(In reply to comment #5)
> Except that they're not "easily override", as the label is not a public
> property, so you cannot (or should not) set the style_class on it.

That's what I was saying. We may want to add support to PopupMenuItem so that you *can* override it, like what you did for the subtitle menu item.

> Secondly, my patch is focused on adding subtitles. Styling them is a matter for
> theme designers (I added bold just to make it clear they're different from the
> rest).

But "styled differently" and "non-reactive" are two completely separate axes in the design. That is, we need to have:

    1. normally-styled menu items that are reactive
    2. differently-styled menu items that are non-reactive
    3. differently-styled menu items that are reactive

It makes more sense to just make it easy to control the style and reactivity of PopupMenuItem, rather than introducing a separate subclass for #2, and then also needing another separate subclass for #3.