The Sound Menu

Merge lp:~3v1n0/indicator-sound/notify-osd-on-scroll into lp:~indicator-applet-developers/indicator-sound/trunk_3

  1. notify-osd-on-scroll
  2. Merge into trunk_3
Proposed by Marco Trevisan (Treviño)
Status: Merged
Merged at revision: 123
Proposed branch: lp:~3v1n0/indicator-sound/notify-osd-on-scroll
Merge into: lp:~indicator-applet-developers/indicator-sound/trunk_3
Diff against target: 202 lines (+98/-4)
4 files modified
configure.ac (+3/-1)
data/com.canonical.indicators.sound.gschema.xml (+9/-0)
src/indicator-sound.c (+84/-3)
src/indicator-sound.h (+2/-0)
To merge this branch: bzr merge lp:~3v1n0/indicator-sound/notify-osd-on-scroll
Reviewer Review Type Date Requested Status
Conor Curran (community) Approve
Matthew Paul Thomas (community) design Approve
Review via email: mp+47189@code.launchpad.net

Description of the change

The indicator-sound indicator has a great usability issue imho: in fact when using the scroll event over it to change the volume level, no feedback is given (just the not-precise-at-all icon change).

Now I know that tooltips can't be show, but showing a notification via notify-osd like when using the volume keys imho is a good way to give precise informations about the current volume level, keeping coherency with the system.

Here how it works (video): http://go.3v1n0.net/hjRrGu

There's just one thing to improve, currently it "reccomends" the installation of notify-osd-icons, if this is has not been done, the "off" volume icon is not shown (I guess the "mute" one should be shown instead), I've tried to make it works anyway, but it doesn't. Look at commit 189 (http://bazaar.launchpad.net/~3v1n0/indicator-sound/notify-osd-on-scroll/revision/189) for more informations.

To post a comment you must log in.
Revision history for this message
TheBeest (thebeest) wrote :

Looks great!

Revision history for this message
Tom Wright (twright-tdw) wrote :

Great idea; this should greatly improve the usability of the sound indicator.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

I was thinking... Maybe you'd prefer to get an option to disable this feature on request, with something like a gconf key...

Revision history for this message
Conor Curran (cjcurran) wrote :

Trevino, nice work.
I'd like to merge this after alpha 2 ( this week ) if that is okay. Some pressing bugs to see to first. A gsettings entry to switch on or off this would definitely help. I merged a big refactor yesterday which means alot your work below needs to be restructured. Not a biggie, we can look at this next week.

Conor

review: Needs Resubmitting
Revision history for this message
Matthew Paul Thomas (mpt) wrote :

I like this design-wise. I have corrected the Notify OSD specification to match. <https://wiki.ubuntu.com/NotifyOSD?action=diff&rev2=205&rev1=203> Thanks for your work, Treviño.

review: Approve (design)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Trevino, nice work.
> I'd like to merge this after alpha 2 ( this week ) if that is okay. Some
> pressing bugs to see to first. A gsettings entry to switch on or off this
> would definitely help.

Ok, I'll see to do that, however I'd keep that setting ON by default, isn't it?

> I merged a big refactor yesterday which means alot your
> work below needs to be restructured. Not a biggie, we can look at this next
> week.

Yes, I saw that, I also had already started to merge my changes.

Revision history for this message
Conor Curran (cjcurran) wrote :

On by the default it fine.

>Yes, I saw that, I also had already started to merge my changes.
great.

Thanks for the nice patch.

Conor

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> On by the default it fine.

Done, the patch is coming to launchpad, feel free to merge it when you want! :)

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> > On by the default it fine.
>
> Done, the patch is coming to launchpad, feel free to merge it when you want!

Ah, remember that packagers should add "notify-osd" and "notify-osd-icons" in the Recommends field...

Revision history for this message
Conor Curran (cjcurran) wrote :

Works nicely Trevino. Thank you !
I might in the morning move the notify code out of the indicator-sound and into the state-manager. Will merge by lunchtime.
thanks again :)
c

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configure.ac'
2--- configure.ac 2011-01-27 03:09:45 +0000
3+++ configure.ac 2011-01-27 17:35:48 +0000
4@@ -35,11 +35,13 @@
5 INDICATOR_DISPLAY_OBJECTS=0.1.11
6 DBUSMENUGLIB_REQUIRED_VERSION=0.3.9
7 GIO_2_0_REQUIRED_VERSION=2.25.13
8+LIBNOTIFY_REQUIRED_VERSION=0.5.0
9
10 PKG_CHECK_MODULES(APPLET, gtk+-2.0 >= $GTK_REQUIRED_VERSION
11 indicator >= $INDICATOR_REQUIRED_VERSION
12 dbusmenu-gtk-0.4 >= $DBUSMENUGTK_REQUIRED_VERSION
13- libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS)
14+ libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS
15+ libnotify >= $LIBNOTIFY_REQUIRED_VERSION)
16
17 AC_SUBST(APPLET_CFLAGS)
18 AC_SUBST(APPLET_LIBS)
19
20=== modified file 'data/com.canonical.indicators.sound.gschema.xml'
21--- data/com.canonical.indicators.sound.gschema.xml 2010-12-16 12:36:43 +0000
22+++ data/com.canonical.indicators.sound.gschema.xml 2011-01-27 17:35:48 +0000
23@@ -25,5 +25,14 @@
24 On start up volume should not be muted.
25 </description>
26 </key>
27+ <key name="show-notify-osd-on-scroll" type="b">
28+ <default>true</default>
29+ <summary>Initial setting for showing notify-osd notification on scroll volume-change</summary>
30+ <description>
31+ When using the mouse scroll-wheel over the indicator-sound icon, the volume changes.
32+ Enabling this setting, every scroll volume-change a notify-osd bubble with the
33+ updated volume value will be shown (if supported by your notification daemon).
34+ </description>
35+ </key>
36 </schema>
37 </schemalist>
38
39=== modified file 'src/indicator-sound.c'
40--- src/indicator-sound.c 2011-01-26 22:40:38 +0000
41+++ src/indicator-sound.c 2011-01-27 17:35:48 +0000
42@@ -27,6 +27,8 @@
43
44 #include <gio/gio.h>
45
46+#include <libnotify/notify.h>
47+
48 #include "indicator-sound.h"
49 #include "transport-widget.h"
50 #include "metadata-widget.h"
51@@ -47,6 +49,8 @@
52 GList* transport_widgets_list;
53 GDBusProxy *dbus_proxy;
54 SoundStateManager* state_manager;
55+ GSettings *settings_manager;
56+ NotifyNotification* notification;
57 };
58
59 #define INDICATOR_SOUND_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), INDICATOR_SOUND_TYPE, IndicatorSoundPrivate))
60@@ -70,6 +74,11 @@
61 gint delta,
62 IndicatorScrollDirection direction);
63
64+//Notification
65+static void indicator_sound_notification_init (IndicatorSound *self);
66+static void indicator_sound_notification_show (IndicatorSound *self,
67+ SoundState state, double value);
68+
69 //key/moust event handlers
70 static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data);
71 static gboolean key_release_cb(GtkWidget* widget, GdkEventKey* event, gpointer data);
72@@ -130,7 +139,11 @@
73 GList* t_list = NULL;
74 priv->transport_widgets_list = t_list;
75 priv->state_manager = g_object_new (SOUND_TYPE_STATE_MANAGER, NULL);
76-
77+ priv->notification = NULL;
78+
79+ priv->settings_manager = g_settings_new("com.canonical.indicators.sound");
80+ indicator_sound_notification_init (self);
81+
82 g_signal_connect ( G_OBJECT(self->service),
83 INDICATOR_SERVICE_MANAGER_SIGNAL_CONNECTION_CHANGE,
84 G_CALLBACK(connection_changed), self );
85@@ -149,6 +162,12 @@
86
87 g_list_free ( priv->transport_widgets_list );
88
89+ g_object_unref(priv->settings_manager);
90+
91+ if (priv->notification) {
92+ notify_uninit();
93+ }
94+
95 G_OBJECT_CLASS (indicator_sound_parent_class)->dispose (object);
96 return;
97 }
98@@ -198,6 +217,33 @@
99 }
100
101 static void
102+indicator_sound_notification_init (IndicatorSound *self)
103+{
104+ IndicatorSoundPrivate* priv = INDICATOR_SOUND_GET_PRIVATE(self);
105+
106+ if (!notify_init(PACKAGE_NAME))
107+ return;
108+
109+ GList* caps = notify_get_server_caps();
110+ gboolean has_notify_osd = FALSE;
111+
112+ if (caps) {
113+ if (g_list_find_custom(caps, "x-canonical-private-synchronous",
114+ (GCompareFunc) g_strcmp0)) {
115+ has_notify_osd = TRUE;
116+ }
117+ g_list_foreach(caps, (GFunc) g_free, NULL);
118+ g_list_free(caps);
119+ }
120+
121+ if (has_notify_osd) {
122+ priv->notification = notify_notification_new(PACKAGE_NAME, NULL, NULL, NULL);
123+ notify_notification_set_hint_string(priv->notification,
124+ "x-canonical-private-synchronous", "");
125+ }
126+}
127+
128+static void
129 connection_changed (IndicatorServiceManager * sm,
130 gboolean connected,
131 gpointer user_data)
132@@ -388,7 +434,11 @@
133 newitem,
134 menu_volume_item,
135 parent);
136- return TRUE;
137+
138+ if (priv->notification)
139+ notify_notification_attach_to_widget(priv->notification, volume_widget);
140+
141+ return TRUE;
142 }
143
144 /*******************************************************************/
145@@ -549,6 +599,34 @@
146 return digested;
147 }
148
149+static void
150+indicator_sound_notification_show(IndicatorSound *self, SoundState state, double value)
151+{
152+ IndicatorSoundPrivate* priv = INDICATOR_SOUND_GET_PRIVATE(self);
153+
154+ if (priv->notification == NULL)
155+ return;
156+
157+ char *icon;
158+ const int notify_value = CLAMP((int)value, -1, 101);
159+
160+ if (state == ZERO_LEVEL) {
161+ // Not available for all the themes
162+ icon = "audio-volume-off";
163+ } else if (state == LOW_LEVEL) {
164+ icon = "audio-volume-low";
165+ } else if (state == MEDIUM_LEVEL) {
166+ icon = "audio-volume-medium";
167+ } else if (state == HIGH_LEVEL) {
168+ icon = "audio-volume-high";
169+ } else {
170+ icon = "audio-volume-muted";
171+ }
172+
173+ notify_notification_update(priv->notification, PACKAGE_NAME, NULL, icon);
174+ notify_notification_set_hint_int32(priv->notification, "value", notify_value);
175+ notify_notification_show(priv->notification, NULL);
176+}
177
178 static void
179 indicator_sound_scroll (IndicatorObject *io, gint delta,
180@@ -575,5 +653,8 @@
181 value -= adj->step_increment;
182 }
183 //g_debug("indicator-sound-scroll - update slider with value %f", value);
184- volume_widget_update(VOLUME_WIDGET(priv->volume_widget), value);
185+ volume_widget_update(VOLUME_WIDGET(priv->volume_widget), value);
186+
187+ if (g_settings_get_boolean(priv->settings_manager, "show-notify-osd-on-scroll"))
188+ indicator_sound_notification_show(INDICATOR_SOUND (io), current_state, value);
189 }
190
191=== modified file 'src/indicator-sound.h'
192--- src/indicator-sound.h 2011-01-24 22:26:21 +0000
193+++ src/indicator-sound.h 2011-01-27 17:35:48 +0000
194@@ -23,6 +23,8 @@
195 You should have received a copy of the GNU General Public License along
196 with this program. If not, see <http://www.gnu.org/licenses/>.
197 */
198+#include "config.h"
199+
200 #include <libindicator/indicator.h>
201 #include <libindicator/indicator-object.h>
202 #include <libindicator/indicator-service-manager.h>

Subscribers

People subscribed via source and target branches