Merge lp:~charlesk/indicator-power/lp-811777 into lp:indicator-power/13.10

[Charles Kerr]

Updates the power indicator to match the spec changes at <https://wiki.ubuntu.com/Power?action=diff&rev2=37&rev1=36>.

This patch is based from hloeung's nice patch, fixes a few edge cases, adds unit tests for labels, headers, & accessible text for all combinations of show time & show percentage, and cleans up the bindings between GSettings and the checkbox actions.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:191
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/51/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-amd64-ci/22
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-armhf-ci/22
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-armhf-ci/22/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-power-ci/51/rebuild

review: Approve (continuous-integration)

[lp:~charlesk/indicator-power/lp-811777]

192. By Charles Kerr on 2013-08-23

copyediting: more descriptive comments when building label/header/a11y text

193. By Charles Kerr on 2013-08-23

copyediting: some newly-dead code should have been removed instead of just commented out

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:193
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/52/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-amd64-ci/23
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-armhf-ci/23
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-saucy-armhf-ci/23/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-power-ci/52/rebuild

review: Approve (continuous-integration)

[Ted Gould (ted)]

Only comment is that this should probably be deleted. Otherwise, cool!

610 + //g_signal_connect (p->settings, "changed::" SETTINGS_SHOW_TIME_S,
611 + // G_CALLBACK(on_show_time_setting_changed), self);

Top approve when that gets dropped.

review: Approve

[Charles Kerr (charlesk)]

heh, looks like we both saw that at the same time :)

> 193. By Charles Kerr 29 minutes ago
> copyediting: some newly-dead code should have been removed instead of just commented out

> Ted Gould (ted) wrote 13 minutes ago:
> Only comment is that this should probably be deleted. Otherwise, cool!

=== modified file 'data/com.canonical.indicator.power.gschema.xml.in'
--- data/com.canonical.indicator.power.gschema.xml.in	2012-02-02 05:00:03 +0000
+++ data/com.canonical.indicator.power.gschema.xml.in	2013-08-23 14:01:50 +0000
@@ -10,6 +10,11 @@
       <_summary>Show time in Menu Bar</_summary>
       <_description>Whether or not to show the time in the menu bar.</_description>
     </key>
+    <key name="show-percentage" type="b">
+      <default>false</default>
+      <_summary>Show percentage in Menu Bar</_summary>
+      <_description>Whether or not to show the percentage in the menu bar.</_description>
+    </key>
     <key enum="icon-policy-enum" name="icon-policy">
       <default>"present"</default>
       <_summary>When to show the battery status in the menu bar.</_summary>
 
=== modified file 'src/device-provider.c'
--- src/device-provider.c	2013-06-19 15:01:19 +0000
+++ src/device-provider.c	2013-08-23 14:01:50 +0000
@@ -54,7 +54,7 @@
  * An easy way to free the list properly in one step is as follows:
  *
  *   g_slist_free_full (list, (GDestroyNotify)g_object_unref);
- * 
+ *
  * Return value: (element-type IndicatorPowerDevice)
  *               (transfer full):
  *               list of devices
 
=== modified file 'src/device.c'
--- src/device.c	2013-06-17 04:16:19 +0000
+++ src/device.c	2013-08-23 14:01:50 +0000
@@ -449,10 +449,11 @@
 ****
 ***/
 
+/* Format time remaining for reading ("H:MM") and speech ("H hours, MM minutes") */
 static void
 get_timestring (guint64   time_secs,
-                gchar   **short_timestring,
-                gchar   **detailed_timestring)
+                gchar   **readable_timestring,
+                gchar   **accessible_timestring)
 {
   gint  hours;
   gint  minutes;
@@ -462,16 +463,16 @@
 
   if (minutes == 0)
     {
-      *short_timestring = g_strdup (_("Unknown time"));
-      *detailed_timestring = g_strdup (_("Unknown time"));
+      *readable_timestring = g_strdup (_("Unknown time"));
+      *accessible_timestring = g_strdup (_("Unknown time"));
 
       return;
     }
 
   if (minutes < 60)
     {
-      *short_timestring = g_strdup_printf ("0:%.2i", minutes);
-      *detailed_timestring = g_strdup_printf (g_dngettext (GETTEXT_PACKAGE, "%i minute",
+      *readable_timestring = g_strdup_printf ("0:%.2i", minutes);
+      *accessible_timestring = g_strdup_printf (g_dngettext (GETTEXT_PACKAGE, "%i minute",
                                               "%i minutes",
                                               minutes), minutes);
       return;
@@ -480,11 +481,11 @@
   hours = minutes / 60;
   minutes = minutes % 60;
 
-  *short_timestring = g_strdup_printf ("%i:%.2i", hours, minutes);
+  *readable_timestring = g_strdup_printf ("%i:%.2i", hours, minutes);
 
   if (minutes == 0)
     {
-      *detailed_timestring = g_strdup_printf (g_dngettext (GETTEXT_PACKAGE, 
+      *accessible_timestring = g_strdup_printf (g_dngettext (GETTEXT_PACKAGE,
                                               "%i hour",
                                               "%i hours",
                                               hours), hours);
@@ -493,7 +494,7 @@
     {
       /* TRANSLATOR: "%i %s %i %s" are "%i hours %i minutes"
        * Swap order with "%2$s %2$i %1$s %1$i if needed */
-      *detailed_timestring = g_strdup_printf (_("%i %s %i %s"),
+      *accessible_timestring = g_strdup_printf (_("%i %s %i %s"),
                                               hours, g_dngettext (GETTEXT_PACKAGE, "hour", "hours", hours),
                                               minutes, g_dngettext (GETTEXT_PACKAGE, "minute", "minutes", minutes));
     }
@@ -561,86 +562,149 @@
   return text;
 }
 
-void
-indicator_power_device_get_time_details (const IndicatorPowerDevice * device,
-                                         gchar ** short_details,
-                                         gchar ** details,
-                                         gchar ** accessible_name)
+static char *
+join_strings (const char * name, const char * time, const char * percent)
+{
+  char * str;
+  const gboolean have_name = name && *name;
+  const gboolean have_time = time && *time;
+  const gboolean have_percent = percent && *percent;
+
+  if (have_name && have_time && have_percent)
+    str = g_strdup_printf (_("%s (%s, %s)"), name, time, percent);
+  else if (have_name && have_time)
+    str = g_strdup_printf (_("%s (%s)"), name, time);
+  else if (have_name && have_percent)
+    str = g_strdup_printf (_("%s (%s)"), name, percent);
+  else if (have_name)
+    str = g_strdup (name);
+  else if (have_time && have_percent)
+    str = g_strdup_printf (_("(%s, %s)"), time, percent);
+  else if (have_time)
+    str = g_strdup_printf (_("(%s)"), time);
+  else if (have_percent)
+    str = g_strdup_printf (_("(%s)"), percent);
+  else
+    str = g_strdup ("");
+
+  return str;
+}
+
+static void
+indicator_power_device_get_text (const IndicatorPowerDevice * device,
+                                 gboolean show_time_in_header,
+                                 gboolean show_percentage_in_header,
+                                 gchar ** header,
+                                 gchar ** label,
+                                 gchar ** a11y)
 {
   if (!INDICATOR_IS_POWER_DEVICE(device))
     {
-      *short_details = NULL;
-      *details = NULL;
-      *accessible_name = NULL;
+      if (a11y != NULL) *a11y = NULL;
+      if (label != NULL) *label = NULL;
+      if (header != NULL) *header = NULL;
       g_warning ("%s: %p is not an IndicatorPowerDevice", G_STRFUNC, device);
       return;
     }
 
   const time_t time = indicator_power_device_get_time (device);
   const UpDeviceState state = indicator_power_device_get_state (device);
+  const UpDeviceKind kind = indicator_power_device_get_kind (device);
+  const gchar * device_name = device_kind_to_localised_string (kind);
   const gdouble percentage = indicator_power_device_get_percentage (device);
-  const UpDeviceKind kind = indicator_power_device_get_kind (device);
-  const gchar * device_name = device_kind_to_localised_string (kind);
+  char pctstr[32] = { '\0' };
+  g_snprintf (pctstr, sizeof(pctstr), "%.0lf%%", percentage);
+
+  GString * terse_time = g_string_new (NULL);
+  GString * verbose_time = g_string_new (NULL);
+  GString * accessible_time = g_string_new (NULL);
 
   if (time > 0)
     {
-      gchar *short_timestring = NULL;
-      gchar *detailed_timestring = NULL;
-
-      get_timestring (time,
-                      &short_timestring,
-                      &detailed_timestring);
+      char * readable_timestr = NULL;
+      char * accessible_timestr = NULL;
+      get_timestring (time, &readable_timestr, &accessible_timestr);
 
       if (state == UP_DEVICE_STATE_CHARGING)
         {
-          /* TRANSLATORS: %2 is a time string, e.g. "1 hour 5 minutes" */
-          *accessible_name = g_strdup_printf (_("%s (%s to charge (%.0lf%%))"), device_name, detailed_timestring, percentage);
-          *details = g_strdup_printf (_("%s (%s to charge)"), device_name, short_timestring);
-          *short_details = g_strdup_printf ("(%s)", short_timestring);
+          g_string_assign (terse_time, readable_timestr);
+          g_string_printf (verbose_time, _("%s to charge"), readable_timestr);
+          g_string_printf (accessible_time, _("%s to charge"), accessible_timestr);
         }
-      else if ((state == UP_DEVICE_STATE_DISCHARGING) && (time > (60*60*12)))
+      else if ((state == UP_DEVICE_STATE_DISCHARGING) && (time <= (60*60*12)))
         {
-          *accessible_name = g_strdup_printf (_("%s"), device_name);
-          *details = g_strdup_printf (_("%s"), device_name);
-          *short_details = g_strdup (short_timestring);
+          g_string_assign (terse_time, readable_timestr);
+          g_string_printf (verbose_time, _("%s left"), readable_timestr);
+          g_string_printf (accessible_time, _("%s left"), accessible_timestr);
         }
       else
         {
-          /* TRANSLATORS: %2 is a time string, e.g. "1 hour 5 minutes" */
-          *accessible_name = g_strdup_printf (_("%s (%s left (%.0lf%%))"), device_name, detailed_timestring, percentage);
-          *details = g_strdup_printf (_("%s (%s left)"), device_name, short_timestring);
-          *short_details = g_strdup (short_timestring);
+          /* if there's more than 12 hours remaining, we don't show it */
         }
 
-      g_free (short_timestring);
-      g_free (detailed_timestring);
+      g_free (readable_timestr);
+      g_free (accessible_timestr);
     }
   else if (state == UP_DEVICE_STATE_FULLY_CHARGED)
     {
-      *details = g_strdup_printf (_("%s (charged)"), device_name);
-      *accessible_name = g_strdup (*details);
-      *short_details = g_strdup ("");
+      g_string_assign (verbose_time,    _("charged"));
+      g_string_assign (accessible_time, _("charged"));
     }
   else if (percentage > 0)
     {
-      /* TRANSLATORS: %2 is a percentage value. Note: this string is only
-       * used when we don't have a time value */
-      *details = g_strdup_printf (_("%s (%.0lf%%)"), device_name, percentage);
-      *accessible_name = g_strdup (*details);
-      *short_details = g_strdup_printf (_("(%.0lf%%)"), percentage);
-    }
-  else if (kind == UP_DEVICE_KIND_LINE_POWER)
-    {
-      *details         = g_strdup (device_name);
-      *accessible_name = g_strdup (device_name);
-      *short_details   = g_strdup ("");
+      g_string_assign (terse_time,      _("estimating…"));
+      g_string_assign (verbose_time,    _("estimating…"));
+      g_string_assign (accessible_time, _("estimating…"));
     }
   else
     {
-      *details = g_strdup_printf (_("%s (not present)"), device_name);
-      *accessible_name = g_strdup (*details);
-      *short_details = g_strdup (_("(not present)"));
+      *pctstr = '\0';
+
+      if (kind != UP_DEVICE_KIND_LINE_POWER)
+        {
+          g_string_assign (verbose_time,    _("not present"));
+          g_string_assign (accessible_time, _("not present"));
+        }
     }
+
+  if (header != NULL)
+    *header = join_strings (NULL,
+                            show_time_in_header ? terse_time->str : "",
+                            show_percentage_in_header ? pctstr : "");
+
+  if (label != NULL)
+    *label  = join_strings (device_name,
+                            verbose_time->str,
+                            NULL);
+
+  if (a11y != NULL)
+    *a11y   = join_strings (device_name,
+                            accessible_time->str,
+                            pctstr);
+
+  g_string_free (terse_time, TRUE);
+  g_string_free (verbose_time, TRUE);
+  g_string_free (accessible_time, TRUE);
+}
+
+gchar *
+indicator_power_device_get_label (const IndicatorPowerDevice * device)
+{
+  gchar * label = NULL;
+  indicator_power_device_get_text (device, FALSE, FALSE,
+                                   NULL, &label, NULL);
+  return label;
+}
+
+void
+indicator_power_device_get_header (const IndicatorPowerDevice * device,
+                                   gboolean                     show_time,
+                                   gboolean                     show_percentage,
+                                   gchar                     ** header,
+                                   gchar                     ** a11y)
+{
+  indicator_power_device_get_text (device, show_time, show_percentage,
+                                   header, NULL, a11y);
 }
 
 /***
 
=== modified file 'src/device.h'
--- src/device.h	2013-06-17 04:16:19 +0000
+++ src/device.h	2013-08-23 14:01:50 +0000
@@ -116,21 +116,22 @@
 IndicatorPowerDevice* indicator_power_device_new_from_variant (GVariant * variant);
 
 
-UpDeviceKind  indicator_power_device_get_kind        (const IndicatorPowerDevice * device);
-UpDeviceState indicator_power_device_get_state       (const IndicatorPowerDevice * device);
-const gchar * indicator_power_device_get_object_path (const IndicatorPowerDevice * device);
-gdouble       indicator_power_device_get_percentage  (const IndicatorPowerDevice * device);
-time_t        indicator_power_device_get_time        (const IndicatorPowerDevice * device);
-
-GStrv         indicator_power_device_get_icon_names  (const IndicatorPowerDevice * device);
-GIcon       * indicator_power_device_get_gicon       (const IndicatorPowerDevice * device);
-
-void          indicator_power_device_get_time_details (const IndicatorPowerDevice * device,
-                                                       gchar ** short_details,
-                                                       gchar ** details,
-                                                       gchar ** accessible_name);
-
-
+UpDeviceKind  indicator_power_device_get_kind         (const IndicatorPowerDevice * device);
+UpDeviceState indicator_power_device_get_state        (const IndicatorPowerDevice * device);
+const gchar * indicator_power_device_get_object_path  (const IndicatorPowerDevice * device);
+gdouble       indicator_power_device_get_percentage   (const IndicatorPowerDevice * device);
+time_t        indicator_power_device_get_time         (const IndicatorPowerDevice * device);
+
+GStrv         indicator_power_device_get_icon_names   (const IndicatorPowerDevice * device);
+GIcon       * indicator_power_device_get_gicon        (const IndicatorPowerDevice * device);
+
+gchar       * indicator_power_device_get_label        (const IndicatorPowerDevice * device);
+
+void          indicator_power_device_get_header       (const IndicatorPowerDevice * device,
+                                                       gboolean                     show_time,
+                                                       gboolean                     show_percentage,
+                                                       gchar                     ** header,
+                                                       gchar                     ** a11y);
 
 
 G_END_DECLS
 
=== modified file 'src/service.c'
--- src/service.c	2013-08-20 03:47:23 +0000
+++ src/service.c	2013-08-23 14:01:50 +0000
@@ -33,6 +33,7 @@
 
 #define SETTINGS_SHOW_TIME_S "show-time"
 #define SETTINGS_ICON_POLICY_S "icon-policy"
+#define SETTINGS_SHOW_PERCENTAGE_S "show-percentage"
 
 G_DEFINE_TYPE (IndicatorPowerService,
                indicator_power_service,
@@ -112,6 +113,7 @@
   GSimpleActionGroup * actions;
   GSimpleAction * header_action;
   GSimpleAction * show_time_action;
+  GSimpleAction * show_percentage_action;
   GSimpleAction * battery_level_action;
   GSimpleAction * brightness_action;
 
@@ -318,16 +320,13 @@
 
   if (p->primary_device != NULL)
     {
-      gchar * details;
-
-      indicator_power_device_get_time_details (p->primary_device,
-                                               &label,
-                                               &details,
-                                               &a11y);
+      indicator_power_device_get_header (p->primary_device,
+                                         g_settings_get_boolean (p->settings, SETTINGS_SHOW_TIME_S),
+                                         g_settings_get_boolean (p->settings, SETTINGS_SHOW_PERCENTAGE_S),
+                                         &label,
+                                         &a11y);
 
       icon = indicator_power_device_get_gicon (p->primary_device);
-
-      g_free (details);
     }
 
   g_variant_builder_init (&b, G_VARIANT_TYPE("a{sv}"));
@@ -337,9 +336,7 @@
 
   if (label != NULL)
     {
-      if (g_settings_get_boolean (p->settings, SETTINGS_SHOW_TIME_S))
-        g_variant_builder_add (&b, "{sv}", "label",
-                               g_variant_new_string (label));
+      g_variant_builder_add (&b, "{sv}", "label", g_variant_new_string (label));
 
       g_free (label);
     }
@@ -376,33 +373,24 @@
 
   if (kind != UP_DEVICE_KIND_LINE_POWER)
   {
-    char * brief;
+    GIcon * icon;
     char * label;
-    char * a11y;
     GMenuItem * menu_item;
-    GIcon * icon = indicator_power_device_get_gicon (device);
-
-    indicator_power_device_get_time_details (device,
-                                             &brief,
-                                             &label,
-                                             &a11y);
-
+
+    icon = indicator_power_device_get_gicon (device);
+    label = indicator_power_device_get_label (device);
     menu_item = g_menu_item_new (label, "indicator.activate-statistics");
 
     if (icon != NULL)
-      {
-        g_menu_item_set_attribute_value (menu_item,
-                                         G_MENU_ATTRIBUTE_ICON,
-                                         g_icon_serialize (icon));
-      }
+      g_menu_item_set_attribute_value (menu_item,
+                                       G_MENU_ATTRIBUTE_ICON,
+                                       g_icon_serialize (icon));
 
     g_menu_append_item (menu, menu_item);
     g_object_unref (menu_item);
 
     g_clear_object (&icon);
-    g_free (brief);
     g_free (label);
-    g_free (a11y);
   }
 }
 
@@ -521,6 +509,10 @@
                  "indicator.show-time");
 
   g_menu_append (menu,
+                 _("Show Percentage in Menu Bar"),
+                 "indicator.show-percentage");
+
+  g_menu_append (menu,
                  _("Power Settings…"),
                  "indicator.activate-settings");
 
@@ -706,49 +698,15 @@
   execute_command ("gnome-power-statistics");
 }
 
-/* FIXME: use a GBinding to tie the gaction's state and the GSetting together? */
-
-static void
-set_show_time_flag (IndicatorPowerService * self, gboolean b)
-{
-  GVariant * v;
-  priv_t * p = self->priv;
-
-  /* update the settings */
-  if (b != g_settings_get_boolean (p->settings, SETTINGS_SHOW_TIME_S))
-    g_settings_set_boolean (p->settings, SETTINGS_SHOW_TIME_S, b);
-
-  /* update the action state */
-  v = g_action_get_state (G_ACTION(p->show_time_action));
-  if (b != g_variant_get_boolean (v))
-    g_simple_action_set_state (p->show_time_action, g_variant_new_boolean (b));
-  g_variant_unref (v);
-
-  rebuild_header_now (self);
-}
-static void
-on_show_time_setting_changed (GSettings * settings, gchar * key, gpointer gself)
-{
-  set_show_time_flag (INDICATOR_POWER_SERVICE(gself),
-                      g_settings_get_boolean (settings, key));
-}
-
-static void
-on_show_time_action_state_changed (GAction    * action,
-                                   GParamSpec * pspec   G_GNUC_UNUSED,
-                                   gpointer     gself)
-{
-  GVariant * v = g_action_get_state (action);
-  set_show_time_flag (INDICATOR_POWER_SERVICE(gself),
-                      g_variant_get_boolean (v));
-  g_variant_unref (v);
-}
+/***
+****
+***/
 
 /* toggles the state */
 static void
-on_show_time_action_activated (GSimpleAction * simple,
-                               GVariant      * parameter G_GNUC_UNUSED,
-                               gpointer        unused    G_GNUC_UNUSED)
+on_toggle_action_activated (GSimpleAction * simple,
+                            GVariant      * parameter G_GNUC_UNUSED,
+                            gpointer        unused    G_GNUC_UNUSED)
 {
   GVariant * v = g_action_get_state (G_ACTION (simple));
   gboolean flag = g_variant_get_boolean (v);
@@ -756,11 +714,27 @@
   g_variant_unref (v);
 }
 
+static gboolean
+settings_to_action_state (GValue    * value,
+                          GVariant  * variant,
+                          gpointer    user_data  G_GNUC_UNUSED)
+{
+  g_value_set_variant (value, variant);
+  return TRUE;
+}
+
+static GVariant *
+action_state_to_settings (const GValue       * value,
+                          const GVariantType * expected_type G_GNUC_UNUSED,
+                          gpointer             user_data     G_GNUC_UNUSED)
+{
+  return g_value_dup_variant (value);
+}
+
 static void
 init_gactions (IndicatorPowerService * self)
 {
   GSimpleAction * a;
-  gboolean show_time;
   priv_t * p = self->priv;
 
   GActionEntry entries[] = {
@@ -777,33 +751,47 @@
 
   /* add the header action */
   a = g_simple_action_new_stateful ("_header", NULL, create_header_state (self));
-  g_simple_action_group_insert (p->actions, G_ACTION(a));
+  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
   p->header_action = a;
 
   /* add the power-level action */
   a = g_simple_action_new_stateful ("battery-level", NULL, g_variant_new_uint32(0));
   g_simple_action_set_enabled (a, FALSE);
-  g_simple_action_group_insert (p->actions, G_ACTION(a));
+  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
   p->battery_level_action = a;
 
   /* add the brightness action */
   a = g_simple_action_new_stateful ("brightness", NULL, action_state_for_brightness (self));
-  g_simple_action_group_insert (p->actions, G_ACTION(a));
+  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
   g_signal_connect (a, "change-state", G_CALLBACK(on_brightness_change_requested), self);
   p->brightness_action = a;
 
   /* add the show-time action */
-  show_time = g_settings_get_boolean (p->settings, SETTINGS_SHOW_TIME_S);
-  a = g_simple_action_new_stateful ("show-time",
-                                    NULL,
-                                    g_variant_new_boolean(show_time));
-  g_signal_connect (a, "activate",
-                    G_CALLBACK(on_show_time_action_activated), self);
-  g_signal_connect (a, "notify",
-                    G_CALLBACK(on_show_time_action_state_changed), self);
-  g_simple_action_group_insert (p->actions, G_ACTION(a));
+  a = g_simple_action_new ("show-time", NULL);
+  g_settings_bind_with_mapping (p->settings, SETTINGS_SHOW_TIME_S,
+                                a, "state",
+                                G_SETTINGS_BIND_DEFAULT,
+                                settings_to_action_state,
+                                action_state_to_settings,
+                                NULL, NULL);
+  g_signal_connect (a, "activate", G_CALLBACK(on_toggle_action_activated), self);
+  g_signal_connect_swapped (a, "notify", G_CALLBACK(rebuild_header_now), self);
+  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
   p->show_time_action = a;
 
+  /* add the show-percentage action */
+  a = g_simple_action_new ("show-percentage", NULL);
+  g_settings_bind_with_mapping (p->settings, SETTINGS_SHOW_PERCENTAGE_S,
+                                a, "state",
+                                G_SETTINGS_BIND_DEFAULT,
+                                settings_to_action_state,
+                                action_state_to_settings,
+                                NULL, NULL);
+  g_signal_connect (a, "activate", G_CALLBACK(on_toggle_action_activated), self);
+  g_signal_connect_swapped (a, "notify", G_CALLBACK(rebuild_header_now), self);
+  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
+  p->show_percentage_action = a;
+
   rebuild_header_now (self);
 }
 
@@ -949,7 +937,7 @@
 {
   IndicatorPowerService * self = INDICATOR_POWER_SERVICE (o);
   priv_t * p = self->priv;
- 
+
   switch (property_id)
     {
       case PROP_DEVICE_PROVIDER:
@@ -1014,10 +1002,16 @@
       g_clear_object (&p->show_time_action);
     }
 
+  if (p->show_percentage_action != NULL)
+    {
+      g_signal_handlers_disconnect_by_data (p->show_percentage_action, self);
+
+      g_clear_object (&p->show_percentage_action);
+    }
+
   g_clear_object (&p->brightness_action);
   g_clear_object (&p->battery_level_action);
   g_clear_object (&p->header_action);
-  g_clear_object (&p->show_time_action);
   g_clear_object (&p->actions);
 
   g_clear_object (&p->conn);
@@ -1051,8 +1045,6 @@
 
   g_signal_connect_swapped (p->settings, "changed::" SETTINGS_ICON_POLICY_S,
                             G_CALLBACK(rebuild_header_now), self);
-  g_signal_connect (p->settings, "changed::" SETTINGS_SHOW_TIME_S,
-                    G_CALLBACK(on_show_time_setting_changed), self);
 
   p->own_id = g_bus_own_name (G_BUS_TYPE_SESSION,
                               BUS_NAME,
 
=== modified file 'tests/test-dbus-listener.cc'
--- tests/test-dbus-listener.cc	2013-01-17 21:07:08 +0000
+++ tests/test-dbus-listener.cc	2013-08-23 14:01:50 +0000
@@ -4,16 +4,16 @@
 Authors:
     Charles Kerr <[e-mail]>
 
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -40,7 +40,7 @@
     int gsd_name_ownership_id;
     int gsd_power_registration_id;
     char * gsd_power_error_string;
-  
+
   protected:
 
     static void
@@ -234,7 +234,7 @@
 
   // cleanup
   g_object_run_dispose (o); // used to get coverage of both branches in the object's dispose func's g_clear_*() calls
-  g_object_unref (o); 
+  g_object_unref (o);
 }
 
 TEST_F(DbusListenerTest, GSDHasNoDevices)
@@ -261,7 +261,7 @@
 
   // cleanup
   g_object_run_dispose (o); // used to get coverage of both branches in the object's dispose func's g_clear_*() calls
-  g_object_unref (o); 
+  g_object_unref (o);
 }
 
 TEST_F(DbusListenerTest, GSDReturnsError)
@@ -285,7 +285,7 @@
 
   // cleanup
   g_object_run_dispose (o); // used to get coverage of both branches in the object's dispose func's g_clear_*() calls
-  g_object_unref (o); 
+  g_object_unref (o);
 }
 
 /* This test emits a PropertiesChanged signal and confirms that
@@ -347,5 +347,5 @@
   ASSERT_EQ (g_slist_length(devices), 2);
 
   // cleanup
-  g_object_unref (o); 
+  g_object_unref (o);
 }
 
=== modified file 'tests/test-device.cc'
--- tests/test-device.cc	2013-06-17 23:01:51 +0000
+++ tests/test-device.cc	2013-08-23 14:01:50 +0000
@@ -4,16 +4,16 @@
 Authors:
     Charles Kerr <[e-mail]>
 
-This program is free software: you can redistribute it and/or modify it 
-under the terms of the GNU General Public License version 3, as published 
+This program is free software: you can redistribute it and/or modify it
+under the terms of the GNU General Public License version 3, as published
 by the Free Software Foundation.
 
-This program is distributed in the hope that it will be useful, but 
-WITHOUT ANY WARRANTY; without even the implied warranties of 
-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR 
+This program is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranties of
+MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
 PURPOSE.  See the GNU General Public License for more details.
 
-You should have received a copy of the GNU General Public License along 
+You should have received a copy of the GNU General Public License along
 with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
@@ -69,27 +69,52 @@
       g_strfreev (names);
     }
 
-    void check_strings (const IndicatorPowerDevice * device,
-                        const char * expected_timestring,
-                        const char * expected_details,
-                        const char * expected_accessible)
-    {
-      char * timestring = NULL;
-      char * details = NULL;
-      char * accessible = NULL;
-
-      indicator_power_device_get_time_details (device, &timestring, &details, &accessible);
-      EXPECT_STREQ (expected_timestring, timestring);
-      EXPECT_STREQ (expected_details, details);
-      EXPECT_STREQ (expected_accessible, accessible);
-
-      g_free (accessible);
-      g_free (details);
-      g_free (timestring);
+    void check_label (const IndicatorPowerDevice * device,
+                      const char * expected_label)
+    {
+      char * label;
+
+      label = indicator_power_device_get_label (device);
+      EXPECT_STREQ (expected_label, label);
+
+      g_free (label);
+    }
+
+    void check_header (const IndicatorPowerDevice * device,
+                       const char * expected_time_and_percent,
+                       const char * expected_time,
+                       const char * expected_percent,
+                       const char * expected_a11y)
+    {
+      char * label;
+      char * a11y;
+
+      indicator_power_device_get_header (device, true, true, &label, &a11y);
+      EXPECT_STREQ (expected_time_and_percent, label);
+      EXPECT_STREQ (expected_a11y, a11y);
+      g_free (label);
+      g_free (a11y);
+
+      indicator_power_device_get_header (device, true, false, &label, &a11y);
+      EXPECT_STREQ (expected_time, label);
+      EXPECT_STREQ (expected_a11y, a11y);
+      g_free (label);
+      g_free (a11y);
+
+      indicator_power_device_get_header (device, false, true, &label, &a11y);
+      EXPECT_STREQ (expected_percent, label);
+      EXPECT_STREQ (expected_a11y, a11y);
+      g_free (label);
+      g_free (a11y);
+
+      indicator_power_device_get_header (device, false, false, &label, &a11y);
+      ASSERT_TRUE (!label || !*label);
+      EXPECT_STREQ (expected_a11y, a11y);
+      g_free (label);
+      g_free (a11y);
     }
 };
 
-
 /***
 ****
 ***/
@@ -266,7 +291,7 @@
       g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
                        INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_EMPTY,
                        NULL);
- 
+
       g_string_append_printf (expected, "%s-empty-symbolic;", kind_str);
       g_string_append_printf (expected, "gpm-%s-empty;", kind_str);
       g_string_append_printf (expected, "gpm-%s-000;", kind_str);
@@ -434,7 +459,7 @@
 
       // state unknown
       g_object_set (o, INDICATOR_POWER_DEVICE_KIND, kind,
-                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN, 
+                       INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_UNKNOWN,
                        NULL);
       g_string_append_printf (expected, "%s-missing-symbolic;", kind_str);
       g_string_append_printf (expected, "gpm-%s-missing;", kind_str);
@@ -456,19 +481,21 @@
   g_setenv ("LANG", "en_US.UTF-8", TRUE);
 
   // bad args: NULL device
-  log_count_ipower_expected++;
-  check_strings (NULL, NULL, NULL, NULL);
+  log_count_ipower_expected += 5;
+  check_label (NULL, NULL);
+  check_header (NULL, NULL, NULL, NULL, NULL);
 
   // bad args: a GObject that isn't a device
-  log_count_ipower_expected++;
+  log_count_ipower_expected += 5;
   GObject * o = G_OBJECT(g_cancellable_new());
-  check_strings ((IndicatorPowerDevice*)o, NULL, NULL, NULL);
+  check_label ((IndicatorPowerDevice*)o, NULL);
+  check_header (NULL, NULL, NULL, NULL, NULL);
   g_object_unref (o);
 
   /**
   ***
   **/
- 
+
   IndicatorPowerDevice * device = INDICATOR_POWER_DEVICE (g_object_new (INDICATOR_POWER_DEVICE_TYPE, NULL));
   o = G_OBJECT(device);
 
@@ -478,9 +505,11 @@
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(60*61),
                    NULL);
-  check_strings (device, "(1:01)",
-                         "Battery (1:01 to charge)",
-                         "Battery (1 hour 1 minute to charge (50%))");
+  check_label (device, "Battery (1:01 to charge)");
+  check_header (device, "(1:01, 50%)",
+                        "(1:01)",
+                        "(50%)",
+                        "Battery (1 hour 1 minute to charge, 50%)");
 
   // discharging, < 12 hours left
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
@@ -488,25 +517,36 @@
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(60*61),
                    NULL);
-  check_strings (device, "1:01",
-                         "Battery (1:01 left)",
-                         "Battery (1 hour 1 minute left (50%))");
+  check_label (device, "Battery (1:01 left)");
+  check_header (device, "(1:01, 50%)",
+                        "(1:01)",
+                        "(50%)",
+                        "Battery (1 hour 1 minute left, 50%)");
 
   // discharging, > 12 hours left
+  // we don't show the clock time when > 12 hours dischargin
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
                    INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_DISCHARGING,
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(60*60*13),
                    NULL);
-  check_strings (device, "13:00", "Battery", "Battery");
+  check_label (device, "Battery");
+  check_header (device, "(50%)",
+                        "",
+                        "(50%)",
+                        "Battery (50%)");
 
-  // fully charged
+// fully charged
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
                    INDICATOR_POWER_DEVICE_STATE, UP_DEVICE_STATE_FULLY_CHARGED,
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 100.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(0),
                    NULL);
-  check_strings (device, "", "Battery (charged)", "Battery (charged)");
+  check_label (device, "Battery (charged)");
+  check_header (device, "(100%)",
+                        "",
+                        "(100%)",
+                        "Battery (charged, 100%)");
 
   // percentage but no time estimate
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
@@ -514,7 +554,11 @@
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 50.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(0),
                    NULL);
-  check_strings (device, "(50%)", "Battery (50%)", "Battery (50%)");
+  check_label (device, "Battery (estimating…)");
+  check_header (device, "(estimating…, 50%)",
+                        "(estimating…)",
+                        "(50%)",
+                        "Battery (estimating…, 50%)");
 
   // no percentage, no time estimate
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_BATTERY,
@@ -522,7 +566,8 @@
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 0.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(0),
                    NULL);
-  check_strings (device, "(not present)", "Battery (not present)", "Battery (not present)");
+  check_label (device, "Battery (not present)");
+  check_header (device, "", "", "", "Battery (not present)");
 
   // power line
   g_object_set (o, INDICATOR_POWER_DEVICE_KIND, UP_DEVICE_KIND_LINE_POWER,
@@ -530,7 +575,8 @@
                    INDICATOR_POWER_DEVICE_PERCENTAGE, 0.0,
                    INDICATOR_POWER_DEVICE_TIME, guint64(0),
                    NULL);
-  check_strings (device, "", "AC Adapter", "AC Adapter");
+  check_label (device, "AC Adapter");
+  check_header (device, "", "", "", "AC Adapter");
 
   // cleanup
   g_object_unref(o);
@@ -605,7 +651,7 @@
           ASSERT_EQ (a, indicator_power_service_choose_primary_device(device_list));
         }
     }
-    
+
   // cleanup
   g_list_free_full (device_list, g_object_unref);
 }
 
=== modified file 'tests/test-service.cc'
--- tests/test-service.cc	2013-06-17 23:01:51 +0000
+++ tests/test-service.cc	2013-08-23 14:01:50 +0000
@@ -65,7 +65,7 @@
     virtual void SetUp()
     {
       ensure_glib_initialized ();
-         
+
       g_setenv( "GSETTINGS_SCHEMA_DIR", SCHEMA_DIR, TRUE);
 
       ac_device = indicator_power_device_new (
@@ -90,7 +90,7 @@
     const char* GetAccessibleDesc (IndicatorPower * power) const
     {
       GList * entries = indicator_object_get_entries (INDICATOR_OBJECT(power));
-      g_assert (g_list_length(entries) == 1); 
+      g_assert (g_list_length(entries) == 1);
       IndicatorObjectEntry * entry = static_cast<IndicatorObjectEntry*>(entries->data);
       const char * ret = entry->accessible_desc;
       g_list_free (entries);
@@ -120,7 +120,7 @@
   devices = g_slist_append (devices, ac_device);
   devices = g_slist_append (devices, battery_device);
   indicator_power_set_devices (power, devices);
-  g_slist_free (devices); 
+  g_slist_free (devices);
 
   g_object_unref (power);
 }