Created attachment 176352 [details] [review]
numerableicon: initial import

GtkNumerableIcon is a GEmblemedIcon subclass with a fixed numeric
emblem, drawn by GTK+ and themable with the icon properties.

Created attachment 176353 [details] [review]
widgetpath: allow GTypes non-derived from GTK_TYPE_WIDGET

This makes things like GtkCellRenderer or GtkNumerableIcon more easily
themeable.

Created attachment 176354 [details] [review]
numerableicon: use CSS properties and GtkStyleContext for theming

Instead of regular properties.
This adds a _new_with_style_context() variant, which makes the icon
optionally themable.

One improvement I'd like to make is to use the background-image property to set the ribbon shape.

Two suggestions:
- the code uses "%u" to format the number; this should be made translatable using C_() so translators can use "%Iu" to get i18n'd digits (btw, why unsigned instead of signed?)
- I still think this should allow setting any text (and named GtkLabelledIcon or something). E.g. this could be used to set roman numerals, or short letter combinations. You mentioned on IRC that it probably looks bad with more than about 2 characters of text, but the docs could just mention that constraint (and/or the code just not show more than that). (Keeping the number property as a shortcut to set text from printf("%u") is fine, of course).

Review of attachment 176353 [details] [review]:

Looks good, please commit

Review of attachment 176354 [details] [review]:

I agree with Christian that it would be useful to allow short strings (1-2 chars) instead of numbers.

::: gtk/gtknumerableicon.c
@@ +79,3 @@
+      g_emblemed_icon_clear_emblems (G_EMBLEMED_ICON (self));
+      return;
+    }

Special cases like this should probably be documented.

@@ +192,3 @@
+    {
+      gtk_widget_path_append_type (path, GTK_TYPE_NUMERABLE_ICON);
+      gtk_style_context_set_path (style, path);

Here you are permanently modifying the passed-in style. I think you probably need to save/restore (though I don't know if save/restore does also save the path, so you may have to do that manually.

@@ +238,3 @@
+    }
+
+  g_clear_object (&self->priv->style);

Always using the newest apis, thats good. :-)

Comment on attachment 176353 [details] [review]
widgetpath: allow GTypes non-derived from GTK_TYPE_WIDGET

Attachment 176353 [details] pushed as b792a31 - widgetpath: allow GTypes non-derived from GTK_TYPE_WIDGET

Created attachment 176601 [details] [review]
numerableicon: add GtkNumerableIcon

GtkNumerableIcon is a GEmblemedIcon subclass with a fixed numeric
emblem, drawn by GTK+ and themable with the icon properties.

Created attachment 176606 [details] [review]
numerableicon: add GtkNumerableIcon

GtkNumerableIcon is a GEmblemedIcon subclass with a fixed numeric
emblem, drawn by GTK+ and themable with the icon properties.

----

Wrong patch attached, sorry for the spam.

New version of the patch (squashed).

Improvements over the last iteration:
- fetch values for "background-image" to allow both actual images and GTK+ gradients as background for the ribbon.
- if we have a style context, use the "font" CSS property to tweak font appearance.
- added gtk_numerable_icon_set_label(), which takes a string. We truncate the string after two characters if it's longer than that.
- turned the unsigned int into an int, and documented the behavior when passing zero.
- added context for translators, and made the format string translatable

I'm not sure I like to change the name of this icon to GtkLabeledIcon, because it doesn't actually let you specify a real label with all of its text properties (like GtkLabel does), just a two-chars string (that's likely a substitute or an alternative representation for a number).

Actual labeled icons, which would look like [1], would be matter for a completely different class I think.

[1] http://trac.adium.im/attachment/ticket/13088/Adium-Dock.jpg

Review of attachment 176606 [details] [review]:

::: docs/reference/gtk/gtk3-sections.txt
@@ +2279,3 @@
+gtk_numerable_icon_get_count
+gtk_numerable_icon_get_style_context
+gtk_numerable_icon_set_count

need to add the label setter and getter here

::: gtk/gtk.symbols
@@ +1583,3 @@
+gtk_numerable_icon_new_with_style_context
+gtk_numerable_icon_set_count
+gtk_numerable_icon_set_style_context

need to add the label getter and setter here

::: gtk/gtknumerableicon.c
@@ +390,3 @@
+
+  buf[4] = '\0';
+  retval = g_utf8_strncpy (buf, label, 2);

I think buf needs to be longer to be on the safe side here.
The docs of g_utf8_strncpy are unfortunately silent on the issue...

@@ +422,3 @@
+
+      self->priv->count = count;
+      self->priv->rendered_string = g_strdup_printf (C_("Number format", "%d"), count);

This probably needs a translators comment in addition to the context to be useful.

@@ +621,3 @@
+ * Note that this is meant for displaying short labels, such as roman numbers,
+ * or single letters, and strings longer than two characters will be truncated.
+ *

The docs need to say something about the priority between label and count.

@@ +661,3 @@
+ *
+ * Sets the currently displayed value of @self to @count.
+ * Note that setting a count of zero removes the emblem.

The docs need to say something about the priority between label and count, and it should be mentioned that the value will be clamped at 99.

Feel free to land this after addressing these comments.

Created attachment 176952 [details] [review]
icon-theme: support GtkNumerable icon emblem sizing

Created attachment 176953 [details] [review]
numerableicon: squashed patch

GtkNumerableIcon is a GEmblemedIcon subclass with a fixed numeric
emblem, drawn by GTK+ and themable with the icon properties.

New version of the patch - the biggest difference is the introduction of two new properties, "background-icon" (GIcon) and "background-icon-name" (string), which allow to specify an icon name (or a GIcon) as background for the rendered emblem.
This allows the use of named icons defined by the theme, which can vary their appearance according to the rendered size (the separate icon theme patch is needed for it to work properly).

Feedback welcome on this approach.

Some more mockups by Jimmac on how this would be used

http://git.gnome.org/browse/gnome-shell-design/plain/mockups/static/app-switcher.png
http://git.gnome.org/browse/gnome-shell-design/plain/mockups/static/notifications-summary.png

Review of attachment 176953 [details] [review]:

Is this patch missing some gtkicontheme.c part to make using numerable icons as emblems actually work ?

::: gtk/gtknumerableicon.c
@@ +876,3 @@
+ * is %NULL, @self will go back using style information or default theming
+ * for its background image.
+ *

Should document the priorities between gicon and icon_name here.

::: gtk/gtknumerableicon.h
@@ +54,3 @@
+
+  /* padding for future class expansion */
+  gpointer padding[16];

This seems a tad excessive, even GtkWidget only has 8 slots of padding atm. I think we generally stick to 8 now.

@@ +81,3 @@
+void              gtk_numerable_icon_set_background_icon_name (GtkNumerableIcon *self,
+                                                               const gchar *icon_name);
+const gchar *     gtk_numerable_icon_get_background_icon_name (GtkNumerableIcon *self);

Should line up the arguments here, to make things look neat.

@@ +85,3 @@
+/* private */
+void _gtk_numerable_icon_set_background_icon_size (GtkNumerableIcon *self,
+                                                   gint icon_size);

I've started to keep private apis out of the public headers now, and moved to separate gtkfooprivate.h headers.

(In reply to comment #18)
> Review of attachment 176953 [details] [review]:
> 
> Is this patch missing some gtkicontheme.c part to make using numerable icons as
> emblems actually work ?

Yes, the gtkicontheme.c part of the patch was attached in comment #14.

I fixed the other comments in the upcoming new patch, thanks for the review.

Created attachment 177122 [details] [review]
numerableicon: squashed patch

GtkNumerableIcon is a GEmblemedIcon subclass with a fixed numeric
emblem, drawn by GTK+ and themable with the icon properties.

Created attachment 177123 [details] [review]
icon-theme: support GtkNumerable icon emblem sizing

Review of attachment 177123 [details] [review]:

What I am missing here is a GTK_IS_NUMERABLE_ICON case in gtk_icon_theme_lookup_by_gicon

Review of attachment 177122 [details] [review]:

Almost there.

Still missing: include the docs section in gtk-docs.sgml, and add SECTION:gtknumerableicon docs in gtknumerableicon.c

::: gtk/gtknumerableicon.c
@@ +747,3 @@
+ * automatically be reset to zero before rendering the label, i.e. the last
+ * method called between #gtk_numerable_icon_set_label and
+ * #gtk_numerable_icon_set_count has always priority.

References to function in gtk-doc look like this: gtk_numerable_icon_set_label()

@@ +871,3 @@
+ * gtk_numerable_icon_set_background_gicon:
+ * @self: a #GtkNumerableIcon
+ * @icon: (allow-none): a #GIcon

Should say 

@icon: (allow-none): a #GIcon, or %NULL

here (ie repeat the annotation in human-digestible form

::: gtk/gtknumerableicon.h
@@ +76,3 @@
+
+void              gtk_numerable_icon_set_background_gicon     (GtkNumerableIcon *self,
+                                                               GIcon *icon);

Indentation is not quite up to GTK+ standards yet, but I can fix that up later.

+static gchar *
+sanitize_label (const gchar *label)
+{
+  gchar buf[128];
+  gchar *retval;
+
+  if (!g_utf8_validate (label, -1, NULL))
+    return NULL;

Why validate? All strings in gtk are UTF-8, we don't generally put too much extra validation in.

+  if (g_utf8_strlen (label, -1) <= 2)
+    return g_strdup (label);
+
+  buf[127] = '\0';
+  retval = g_utf8_strncpy (buf, label, 2);
+
+  return g_strdup (retval);
+}

This doesn't look right. You may be correct in limiting the shown text to 2 glyphs, but that does not correspond to just 2 utf-8 codepoints. The code above will not work correctly, since it e.g. will cut off combining accent characters.

Also, since you explicitly mention roman numerals in the docs, making it impossible to actually put 3 (III) or 7 (VII) on the emblem (which in glyph size is probably not longer than most 2 letter sequences), i.e. not even supportging all numbers from 1 to 10, seems suboptimal :)

Actually I forgot about U+2060..U+216B, so the last point can be worked around by using these real roman numbers instead of faking them with ascii :) The point about combining characters still stands, however.

Committed after a round of cleanups and changes.