[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center Importance: Unknown = Wishlist -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center Status: Unknown = Fix Released -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Branch linked: lp:ubuntu/gnome-control-center -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
This bug was fixed in the package gnome-control-center - 1:2.29.90-0ubuntu2 --- gnome-control-center (1:2.29.90-0ubuntu2) lucid; urgency=low * debian/patches/02_use_application_indicator.patch: - use the lucid application indicator, thanks Travis B. Hartwell (lp: #497857) * debian/patches/99_autoreconf.patch: - refreshed for the previous change to build -- Sebastien Bacher seb...@ubuntu.com Thu, 11 Feb 2010 15:10:32 +0100 ** Changed in: gnome-control-center (Ubuntu) Status: Fix Committed = Fix Released -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center (Ubuntu) Assignee: Ken VanDine (ken-vandine) = Sebastien Bacher (seb128) -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
Thank you for your work there, some comment:
GtkStatusIcon *icon;
-
Why the blank line change?
+#ifdef HAVE_APP_INDICATOR
+const char TYPING_MONITOR_ACTIVE_ICON[] = bar-green;
+const char TYPING_MONITOR_ATTENTION_ICON[] = bar-red;
you can use #define TYPING_MONITOR_ACTIVE_ICON bar-green rather,
that's what GNOME does usually
+static gint get_time_left (DrWright
*drwright);
this change seems to be code cleaning
+update_app_indicator (DrWright *dr)
+ current_status = app_indicator_get_status (dr-indicator);
+ if (new_status != current_status) {
no need to get the current status and compare to the new one in this
function, could can just call set_status on the new one, if there is no
status change that will just do nothing
+#ifndef HAVE_APP_INDICATOR
gtk_status_icon_set_from_pixbuf (dr-icon,
dr-composite_bar);
+#endif
} else {
+#ifndef HAVE_APP_INDICATOR
gtk_status_icon_set_from_pixbuf (dr-icon,
dr-neutral_bar);
+#endif
no need to have 2 ifndef there, you can as well use one since there no
action to do in either case
stop_blinking (dr);
+
the new line adding there doesn't seem required
+ const gchar normal_msg_template[]= Take a break now (next in
%dm);
+ const gchar less_than_one_msg_template[] = Take a break now (next in
less than one minute);
why do you use strings different than the upstream ones? the upstream
labels indicate that the indication is the time before next break where
you state take a break now, did you change how the software works?
aren't just menu indications about the next break? in that's the case
where should the user take a break now when looking when is the next
one?
is ngettext() required there is the singular and plurial forms have no
change?
+get_time_left (DrWright *dr)
+{
+ gint elapsed_time, min;
the min variable seems not required there, you can as well use return
directly the value
init_tray_icon (dr);
-
g_timeout_add_seconds (12,
the blank line change is not required
--
Support application indicators
https://bugs.launchpad.net/bugs/497857
You received this bug notification because you are a member of Ubuntu
Desktop Bugs, which is subscribed to gnome-control-center in ubuntu.
--
desktop-bugs mailing list
desktop-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
Thank you for your work there, some comment:
GtkStatusIcon *icon;
-
Why the blank line change?
These were probably all unintentional, I have since found a parameter
with git diff that allows me to ignore whitespace changes when
creating diffs that I will use. They partly came from calling M-x
delete-trailing-whitespace in Emacs, which possibly removed whitespace
that was there in the original code. I'll make sure there are no
extraneous lines in future patches.
+#ifdef HAVE_APP_INDICATOR
+const char TYPING_MONITOR_ACTIVE_ICON[] = bar-green;
+const char TYPING_MONITOR_ATTENTION_ICON[] = bar-red;
you can use #define TYPING_MONITOR_ACTIVE_ICON bar-green rather, that's
what GNOME does usually
Good hint, thanks.
+static gint get_time_left (DrWright *drwright);
this change seems to be code cleaning
I did this so I didn't copy and paste code between the two methods
that needed this functionality between the old update_tooltip () and
the new update_menu_text () functionality.
+update_app_indicator (DrWright *dr)
+current_status = app_indicator_get_status (dr-indicator);
+if (new_status != current_status) {
no need to get the current status and compare to the new one in this function,
could can just call set_status on the new one, if there is no status change
that will just do nothing
Good to know, I noticed you asking this question earlier, I'll remove
the extraneous check.
+#ifndef HAVE_APP_INDICATOR
gtk_status_icon_set_from_pixbuf (dr-icon,
dr-composite_bar);
+#endif
} else {
+#ifndef HAVE_APP_INDICATOR
gtk_status_icon_set_from_pixbuf (dr-icon,
dr-neutral_bar);
+#endif
no need to have 2 ifndef there, you can as well use one since there no action
to do in either case
Oh duh. :)
stop_blinking (dr);
+
the new line adding there doesn't seem required
+const gchar normal_msg_template[] = Take a break now (next in %dm);
+const gchar less_than_one_msg_template[] = Take a break now (next in
less than one minute);
why do you use strings different than the upstream ones? the upstream labels
indicate that the indication is the time before next break where you state
take a break now, did you change how the software works? aren't just menu
indications about the next break? in that's the case where should the user
take a break now when looking when is the next one?
This was from a suggestion from mpt. Previously, the information on
how much time left was indicated via a tooltip, which application
indicators don't support.
To quote from an e-mail from mpt:
-- quote --
For the typing break in particular, I suggest including the time in the
Take a Break item, like this:
..
| Take a Break Now (next in 7m) |
||
| Settings… |
| About Typing Break |
''
--- end quote ---
The reason we shortened the text there is the previous strings were
really long and would have made the menu strangely long. I tried not
to change the way the software works, just the way in which the
current information is conveyed. Would be it be clearer if the time
left was an insensitive menu item alone at the very top of the menu?
is ngettext() required there is the singular and plurial forms have no change?
Probably not, I bet I could just use the _N() macro.
+get_time_left (DrWright *dr)
+{
+gint elapsed_time, min;
the min variable seems not required there, you can as well use return directly
the value
Good catch, I'll fix that.
init_tray_icon (dr);
-
g_timeout_add_seconds (12,
the blank line change is not required
As noted above, whoops.
--
Support application indicators
https://bugs.launchpad.net/bugs/497857
You received this bug notification because you are a member of Ubuntu
Desktop Bugs, which is subscribed to gnome-control-center in ubuntu.
--
desktop-bugs mailing list
desktop-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Tags added: patch -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
Attached is an updated patch: 1) following recommendations from Sebastien above 2) updating the configure.ac with the latest suggestions from Ken ** Patch added: Patch for app indicators, take 2 http://launchpadlibrarian.net/39031554/app-indicators-take2.patch -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
Hopefully final changes, based on what Sebastien pointed out on IRC, and another thing I noticed: 1) preprocessed out the code to blink completely in the case of using app indicators 2) the #ifdef/#endif's sometimes were separated by a bit, so I added a comment on each #endif to be #endif /* HAVE_APP_INDICATOR */ ?field.comment=Hopefully final changes, based on what Sebastien pointed out on IRC, and another thing I noticed: 1) preprocessed out the code to blink completely in the case of using app indicators 2) the #ifdef/#endif's sometimes were separated by a bit, so I added a comment on each #endif to be #endif /* HAVE_APP_INDICATOR */ Should be ready for upstream. ** Patch added: Take 3 http://launchpadlibrarian.net/39033229/app-indicators-take3.patch -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
Note: As agreed on with jcastro, this only supports showing green when plenty of time before locked - showing red right before - locking. The original behavior shows gradients of green as time progresses and then flashing red right before locking, then red when locked. ** Patch added: Support for application indicators http://launchpadlibrarian.net/38931559/app-indicators.patch ** Changed in: gnome-control-center (Ubuntu) Status: In Progress = Fix Committed ** Changed in: gnome-control-center (Ubuntu) Assignee: Travis B. Hartwell (nafai) = Ken VanDine (ken-vandine) -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center (Ubuntu) Assignee: Canonical Desktop Experience Team (canonical-dx-team) = Travis B. Hartwell (nafai) -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center (Ubuntu) Status: Triaged = In Progress -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Bug watch added: GNOME Bug Tracker #606671 https://bugzilla.gnome.org/show_bug.cgi?id=606671 ** Also affects: gnome-control-center via https://bugzilla.gnome.org/show_bug.cgi?id=606671 Importance: Unknown Status: Unknown -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
[Bug 497857] Re: Support application indicators
** Changed in: gnome-control-center (Ubuntu) Importance: Undecided = Wishlist ** Changed in: gnome-control-center (Ubuntu) Status: New = Triaged -- Support application indicators https://bugs.launchpad.net/bugs/497857 You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is subscribed to gnome-control-center in ubuntu. -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs
