Created attachment 332178 [details] [review]
shortcut-label: make it public

GtkShortcutLabel is a widget that displays a single
shortcut accelerator or gesture in the user interface,
and is currently used by the shortcuts window.

This widget, however, has public value as other applications
also may want to expose their own shortcuts. For instance,
it'll be useful for the Keyboard panel on Control Center and
the new shortcut editor in Pitivi, among others.

This patch exposes GtkShortcutLabel as a public widget,
and adds the necessary documentation.

Created attachment 332179 [details] [review]
shortcut-label: add 'unset-text' property

When there's no useful shortcut accelerator set,
GtkShortcutLabel doesn't show any useful information.

To work around that, add a new property to set the
text to be displayed when there's no accelerator
available.

Created attachment 332215 [details] [review]
shortcut-label: add 'unset-text' property

The previous patch was missing a g_free() at finalize(). Fixed in this version.

(In reply to Georges Basile Stavracas Neto from comment #0)
> Various components around GNOME will benefit from
> using the GtkShortcutLabel widget. The following
> patches adapt it to be public.

What components are those, other that g-c-c ?

Review of attachment 332178 [details] [review]:

::: gtk/gtkshortcutlabel.c
@@ +450,3 @@
 
+  /**
+   * Gtkshortcutlabel:accelerator:

I think you need to fix up the capitalization here for gtk-doc to pick this up

@@ +453,3 @@
+   *
+   * The accelerator that @self displays.
+   *

This is missing the important information: whats the format ?

Should copy (or refer to) the docs for GtkShortcutsShortcut:accelerator

Review of attachment 332215 [details] [review]:

::: gtk/gtkshortcutlabel.c
@@ +379,3 @@
+    {
+      gtk_container_add (GTK_CONTAINER (self), dim_label (self->unset_text));
+      gtk_widget_show_all (GTK_WIDGET (self));

This should probably only show the newly added label. The visibility of the shortcutlabel itself is under the users control and you shouldn't touch it.

@@ +477,3 @@
 
+  /**
+   * Gtkshortcutlabel:unset-text:

Capitalization again

@@ +486,3 @@
+    g_param_spec_string ("accelerator", P_("Accelerator"), P_("Accelerator"),
+                         NULL,
+                         (G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));

Copy-paste mistake here - the doc comment speaks about unset-text, but the property is PROP_ACCELERATOR

@@ +559,3 @@
+/**
+ * gtk_shortcut_label_get_unset_text:
+ * @self: a #Gtkshortcutlabel

Capitalization

@@ +579,3 @@
+ * gtk_shortcut_label_set_unset_text:
+ * @self: a #GtkShortcutLabel
+ * @unset_text: the new accelerator

copy-paste

::: gtk/gtkshortcutlabel.h
@@ +53,3 @@
+GDK_AVAILABLE_IN_3_22
+void         gtk_shortcut_label_set_unset_text  (GtkShortcutLabel *self,
+                                                 const gchar      *unset_text);

I must say, seeing that function name ("set_unset"), the naming choice is unfortunate (despite being my suggestion...)

This was one of the worst patchsets I've ever published. What a shame. I'm working on the corrections.

(In reply to Matthias Clasen from comment #4)

> What components are those, other that g-c-c ?

There's a Pitivi intern working on a new custom shortcut editor which will use this widget.

(In reply to Matthias Clasen from comment #4)
> What components are those, other that g-c-c ?

I'm going to use it in gnome-terminal's preferences dialogue (was going with a copypasted version, but making it public in gtk+ is preferable).

I was expecting that a shortcut_editor_ would be a much more interesting widget to share, actually. In fact, there is a patch somewhere in bugzilla for such a widget.

Created attachment 332236 [details] [review]
shortcut-label: make it public

Corrected. I opted to refer the other property, to avoid possible future inconsistencies between docs.

Created attachment 332237 [details] [review]
shortcut-label: add 'disabled-text' property

All the points were corrected, and I renamed the property to 'disabled-text' to avoid weird function names.

Review of attachment 332236 [details] [review]:

Looks good now

Review of attachment 332237 [details] [review]:

Feel free to push with these minor fixes applied

::: gtk/gtkshortcutlabel.c
@@ +564,3 @@
+/**
+ * gtk_shortcut_label_get_disabled_text:
+ * @self: a #Gtkshortcutlabel

Capitalization: GtkShortcutLabel

@@ +584,3 @@
+ * gtk_shortcut_label_set_disabled_text:
+ * @self: a #GtkShortcutLabel
+ * @unset_text: the text to be displayed when no accelerator is set

Should be @disabled_text now

Thanks for the reviews!

Attachment 332236 [details] pushed as 7543cd8 - shortcut-label: make it public
Attachment 332237 [details] pushed as ddee89f - shortcut-label: add 'disabled-text' property