Merge lp:~wingpanel-devs/audience/sound-indicator-in-fullscreen into lp:~audience-members/audience/trunk

Proposed by Marcus Wichelmann on 2015-10-22
Status: Rejected
Rejected by: Zisu Andrei on 2016-11-06
Proposed branch: lp:~wingpanel-devs/audience/sound-indicator-in-fullscreen
Merge into: lp:~audience-members/audience/trunk
Diff against target: 196 lines (+90/-1)
4 files modified
src/CMakeLists.txt (+3/-1)
src/Widgets/BottomBar.vala (+16/-0)
src/Widgets/IndicatorContainer.vala (+62/-0)
src/Widgets/PlayerPage.vala (+9/-0)
To merge this branch: bzr merge lp:~wingpanel-devs/audience/sound-indicator-in-fullscreen
Reviewer Review Type Date Requested Status
Viko Adi Rahmawan 2015-10-22 Disapprove on 2015-11-06
Review via email: mp+275313@code.launchpad.net

Commit message

Show sound indicator in fullscreen mode

Description of the change

Use the new wingpanel library to show a sound indicator in fullscreen mode like requested in #953272.

To post a comment you must log in.
Viko Adi Rahmawan (vikoadi) wrote :

depending on wingpanel sounds overkill
can we just copy the code?

Marcus Wichelmann (l-admin-3) wrote :

Of course we can copy the code, but duplicated code is imo never the right choice.
Why is depending on wingpanel overkill? The wingpanel api is built for being able to be integrated into different parts of the desktop enviroment. ;-)

Viko Adi Rahmawan (vikoadi) wrote :

i think the inclusion of wingpanel itself just for sound indicator is just too much, beside that audience dont have control over it, like we dont need the mpris, setting link and cant remove it.
Also we the hide observer cant get the status, is it toggle or shown

Marcus Wichelmann (l-admin-3) wrote :

The sound indigator is able to decide which contents are shown for audience.

Viko Adi Rahmawan (vikoadi) wrote :

i still got mpris and setting link here.
how to hide it?

Viko Adi Rahmawan (vikoadi) wrote :

otherwise i still think depending on wingpanel is a bad move as the dependency will be coupled together. if i want to have audience, then i need wingpanel, then wingpanel need gala, then gala need plank. i would go for minimum dependency as possible espessially if it only use in small portion of apps.
to overcome code duplication, if we can upload a stable indicator-sound tar.gz, we can script cmake to download and extract those tar.gz then we can simply include the class we want. of course its depend on how loosely sound indicator class

review: Disapprove
Adam BieĊ„kowski (donadigo) wrote :

As Viko said there shouldn't be a strict dependency on wingpanel. I suggest to define cmake variable, something like #HAS_WINGPANEL and then check it in the code.

Viko Adi Rahmawan (vikoadi) wrote :

check this out guys https://code.launchpad.net/~vikoadi/audience/volume-control
it need a compressed wingpanel-indicator-sound.tar.gz uploaded somewhere, which usually are done when its released. for the time being I just compressed sound indicator into wingpanel-indicator-sound-0.1.tar.gz and put it audience project directory. bzr will handle downloading and decompressing.

maybe we can upload an alpha tar.gz somewhere?

Marcus Wichelmann (l-admin-3) wrote :

Hi, I don't see how that fixes the problem. You're still duplicating code and since it looked like you added the wingpanel-indicator-sound dependency, wingpanel is a dependency, too.
Please explain what exactly the code in src/Volume is for and why that isn't a duplication.

Viko Adi Rahmawan (vikoadi) wrote :

the wingpanel-indicator-sound refer to cmake target that is created using ExternalProject_Add, not sound indicator package. It can be compiled event without libwingpanel installed.

So those ExternalProject_Add will download and extract the tar.gz file into wingpanel-indicator-sound folder. Then we can add the class that we need into vala_precompile package. Here i can only reuse Sound.Widget.Scale, Service.VolumeControl, and Service.Setting. I need to have a controller to wire up Widget.Scale view into VolumeControl model, thats why i need to write all that src/Volume class.

If we can abstract out the volume controller part of Indicator.vala into its own class, we can always reuse it in Audience.

Unmerged revisions

550. By Marcus Wichelmann on 2015-10-22

Show sound indicator in fullscreen mode

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt 2015-09-04 11:10:22 +0000
+++ src/CMakeLists.txt 2015-10-22 11:44:48 +0000
@@ -11,7 +11,7 @@
11# pkgconfig, real C code11# pkgconfig, real C code
12find_package (PkgConfig)12find_package (PkgConfig)
1313
14set (PKG_DEPS granite>=0.3.0 clutter-gtk-1.0 gstreamer-1.0 gstreamer-pbutils-1.0 gstreamer-video-1.0 gstreamer-tag-1.0)14set (PKG_DEPS granite>=0.3.0 clutter-gtk-1.0 gstreamer-1.0 gstreamer-pbutils-1.0 gstreamer-video-1.0 gstreamer-tag-1.0 wingpanel-2.0)
15set (VALA_DEPS15set (VALA_DEPS
16 granite>=0.3.016 granite>=0.3.0
17 clutter-gtk-1.017 clutter-gtk-1.0
@@ -20,6 +20,7 @@
20 gstreamer-pbutils-1.020 gstreamer-pbutils-1.0
21 gstreamer-video-1.021 gstreamer-video-1.0
22 gstreamer-tag-1.022 gstreamer-tag-1.0
23 wingpanel-2.0
23)24)
2425
25pkg_check_modules (DEPS REQUIRED ${PKG_DEPS})26pkg_check_modules (DEPS REQUIRED ${PKG_DEPS})
@@ -65,6 +66,7 @@
65 Widgets/VideoPlayer.vala66 Widgets/VideoPlayer.vala
66 Widgets/WelcomePage.vala67 Widgets/WelcomePage.vala
67 Widgets/PlayerPage.vala68 Widgets/PlayerPage.vala
69 Widgets/IndicatorContainer.vala
68 PACKAGES70 PACKAGES
69 ${VALA_DEPS}71 ${VALA_DEPS}
70 OPTIONS72 OPTIONS
7173
=== modified file 'src/Widgets/BottomBar.vala'
--- src/Widgets/BottomBar.vala 2015-06-14 07:02:09 +0000
+++ src/Widgets/BottomBar.vala 2015-10-22 11:44:48 +0000
@@ -34,6 +34,7 @@
34 private Gtk.Button play_button;34 private Gtk.Button play_button;
35 private Gtk.Button preferences_button;35 private Gtk.Button preferences_button;
36 private Gtk.Revealer unfullscreen_revealer;36 private Gtk.Revealer unfullscreen_revealer;
37 private Gtk.Revealer indicator_revealer;
37 private bool is_playing = false;38 private bool is_playing = false;
38 private uint hiding_timer = 0;39 private uint hiding_timer = 0;
3940
@@ -91,8 +92,10 @@
91 notify["fullscreen"].connect (() => {92 notify["fullscreen"].connect (() => {
92 if (fullscreen == true && child_revealed == true) {93 if (fullscreen == true && child_revealed == true) {
93 unfullscreen_revealer.set_reveal_child (true);94 unfullscreen_revealer.set_reveal_child (true);
95 indicator_revealer.set_reveal_child (true);
94 } else if (fullscreen == false && child_revealed == true) {96 } else if (fullscreen == false && child_revealed == true) {
95 unfullscreen_revealer.set_reveal_child (false);97 unfullscreen_revealer.set_reveal_child (false);
98 indicator_revealer.set_reveal_child (false);
96 }99 }
97 });100 });
98101
@@ -123,6 +126,16 @@
123 return unfullscreen_revealer;126 return unfullscreen_revealer;
124 }127 }
125128
129 public Gtk.Revealer get_indicator_container () {
130 indicator_revealer = new Gtk.Revealer ();
131 indicator_revealer.opacity = GLOBAL_OPACITY;
132 indicator_revealer.transition_type = Gtk.RevealerTransitionType.CROSSFADE;
133 var indicator_container = new IndicatorContainer ();
134 indicator_revealer.add (indicator_container);
135 indicator_revealer.show_all ();
136 return indicator_revealer;
137 }
138
126 public void toggle_play_pause () {139 public void toggle_play_pause () {
127 is_playing = !is_playing;140 is_playing = !is_playing;
128 if (is_playing == true) {141 if (is_playing == true) {
@@ -140,8 +153,10 @@
140 base.set_reveal_child (reveal);153 base.set_reveal_child (reveal);
141 if (reveal == true && fullscreen == true) {154 if (reveal == true && fullscreen == true) {
142 unfullscreen_revealer.set_reveal_child (reveal);155 unfullscreen_revealer.set_reveal_child (reveal);
156 indicator_revealer.set_reveal_child (reveal);
143 } else if (reveal == false) {157 } else if (reveal == false) {
144 unfullscreen_revealer.set_reveal_child (reveal);158 unfullscreen_revealer.set_reveal_child (reveal);
159 indicator_revealer.set_reveal_child (reveal);
145 }160 }
146 }161 }
147162
@@ -174,6 +189,7 @@
174 }189 }
175 set_reveal_child (false);190 set_reveal_child (false);
176 unfullscreen_revealer.set_reveal_child (false);191 unfullscreen_revealer.set_reveal_child (false);
192 indicator_revealer.set_reveal_child (false);
177 hiding_timer = 0;193 hiding_timer = 0;
178 return false;194 return false;
179 });195 });
180196
=== added file 'src/Widgets/IndicatorContainer.vala'
--- src/Widgets/IndicatorContainer.vala 1970-01-01 00:00:00 +0000
+++ src/Widgets/IndicatorContainer.vala 2015-10-22 11:44:48 +0000
@@ -0,0 +1,62 @@
1/*
2 * Copyright (c) 2011-2015 Marcus Wichelmann (marcus.wichelmann@hotmail.de)
3 *
4 * This program is free software; you can redistribute it and/or
5 * modify it under the terms of the GNU General Public
6 * License as published by the Free Software Foundation; either
7 * version 2 of the License, or (at your option) any later version.
8 *
9 * This program is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12 * General Public License for more details.
13 *
14 * You should have received a copy of the GNU General Public
15 * License along with this program; if not, write to the
16 * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
17 * Boston, MA 02111-1307, USA.
18 */
19
20public class Audience.Widgets.IndicatorContainer : Gtk.Grid {
21 public IndicatorContainer () {
22 Wingpanel.IndicatorManager indicator_manager = Wingpanel.IndicatorManager.get_default ();
23 indicator_manager.initialize (Wingpanel.IndicatorManager.ServerType.SESSION);
24
25 if (!indicator_manager.has_indicators ()) {
26 return;
27 }
28
29 foreach (Wingpanel.Indicator indicator in indicator_manager.get_indicators ()) {
30 if (indicator.code_name == Wingpanel.Indicator.SOUND) {
31 add_indicator (indicator);
32 }
33 }
34 }
35
36 private void add_indicator (Wingpanel.Indicator indicator) {
37 Gtk.Widget display_widget = indicator.get_display_widget ();
38 display_widget.add_events (Gdk.EventMask.BUTTON_PRESS_MASK | Gdk.EventMask.SCROLL_MASK);
39 display_widget.valign = Gtk.Align.START;
40
41 Gtk.Popover popover = new Gtk.Popover (display_widget);
42 popover.add (indicator.get_widget ());
43
44 display_widget.button_press_event.connect ((e) => {
45 if ((e.button == Gdk.BUTTON_PRIMARY || e.button == Gdk.BUTTON_SECONDARY) && e.type == Gdk.EventType.BUTTON_PRESS) {
46 if (popover.get_visible ()) {
47 popover.hide ();
48 } else {
49 popover.show_all ();
50 }
51
52 return Gdk.EVENT_STOP;
53 }
54
55 display_widget.button_press_event (e);
56
57 return Gdk.EVENT_PROPAGATE;
58 });
59
60 this.add (display_widget);
61 }
62}
0\ No newline at end of file63\ No newline at end of file
164
=== modified file 'src/Widgets/PlayerPage.vala'
--- src/Widgets/PlayerPage.vala 2015-09-12 20:11:41 +0000
+++ src/Widgets/PlayerPage.vala 2015-10-22 11:44:48 +0000
@@ -14,6 +14,8 @@
14 private Clutter.Stage stage;14 private Clutter.Stage stage;
15 private Gtk.Revealer unfullscreen_bar;15 private Gtk.Revealer unfullscreen_bar;
16 private GtkClutter.Actor unfullscreen_actor;16 private GtkClutter.Actor unfullscreen_actor;
17 private Gtk.Revealer indicator_bar;
18 private GtkClutter.Actor indicator_actor;
17 private GtkClutter.Actor bottom_actor;19 private GtkClutter.Actor bottom_actor;
18 private GnomeMediaKeys mediakeys;20 private GnomeMediaKeys mediakeys;
1921
@@ -56,6 +58,7 @@
56 bottom_bar.set_repeat (false);58 bottom_bar.set_repeat (false);
5759
58 unfullscreen_bar = bottom_bar.get_unfullscreen_button ();60 unfullscreen_bar = bottom_bar.get_unfullscreen_button ();
61 indicator_bar = bottom_bar.get_indicator_container ();
5962
60 bottom_actor = new GtkClutter.Actor.with_contents (bottom_bar);63 bottom_actor = new GtkClutter.Actor.with_contents (bottom_bar);
61 bottom_actor.opacity = GLOBAL_OPACITY;64 bottom_actor.opacity = GLOBAL_OPACITY;
@@ -65,6 +68,10 @@
65 unfullscreen_actor.opacity = GLOBAL_OPACITY;68 unfullscreen_actor.opacity = GLOBAL_OPACITY;
66 stage.add_child (unfullscreen_actor);69 stage.add_child (unfullscreen_actor);
6770
71 indicator_actor = new GtkClutter.Actor.with_contents (indicator_bar);
72 indicator_actor.opacity = GLOBAL_OPACITY;
73 stage.add_child (indicator_actor);
74
68 this.size_allocate.connect (on_size_allocate);75 this.size_allocate.connect (on_size_allocate);
69 App.get_instance ().mainwindow.key_press_event.connect (on_key_press_event);76 App.get_instance ().mainwindow.key_press_event.connect (on_key_press_event);
70 App.get_instance ().mainwindow.window_state_event.connect (on_window_state_event);77 App.get_instance ().mainwindow.window_state_event.connect (on_window_state_event);
@@ -338,6 +345,8 @@
338 bottom_actor.y = stage.get_height () - bottom_bar_size;345 bottom_actor.y = stage.get_height () - bottom_bar_size;
339 unfullscreen_actor.y = 6;346 unfullscreen_actor.y = 6;
340 unfullscreen_actor.x = stage.get_width () - bottom_bar_size - 6;347 unfullscreen_actor.x = stage.get_width () - bottom_bar_size - 6;
348 indicator_actor.y = 6;
349 indicator_actor.x = 6;
341 }350 }
342351
343 public bool update_pointer_position (double y, int window_height) {352 public bool update_pointer_position (double y, int window_height) {

Subscribers

People subscribed via source and target branches