Added Charles suggestions

2 files changed, 41 insertions, 20 deletions

diff --git a/src/sound-menu.vala b/src/sound-menu.vala
index 896b7d2..669e83d 100644
--- a/src/sound-menu.vala
+++ b/src/sound-menu.vala
@@ -29,7 +29,7 @@ public class SoundMenu: Object
 		HIDE_INACTIVE_PLAYERS_PLAY_CONTROLS = 32
 	}
 
-	public enum PlayerSectionPosistion {
+	public enum PlayerSectionPosition {
 		LABEL = 0,
 		PLAYER_CONTROLS = 1,
 		PLAYLIST = 2
@@ -182,21 +182,12 @@ public class SoundMenu: Object
 				if (index == -1) {
 					this.insert_player_section (player);
 				}
-				else {
-					update_player_section (player, index);
-				}
 				number_of_running_players++;
 			}
 			else {
 				number_of_running_players--;
 				if (this.hide_inactive)
 					this.remove_player_section (player);
-				else {
-					if (index != -1) {
-						// just update to verify if we must hide the player controls
-						update_player_section (player, index);
-					}
-				}
 			}
 			this.update_playlists (player);
 
@@ -307,7 +298,7 @@ public class SoundMenu: Object
 
 	MenuItem create_playback_menu_item (MediaPlayer player) {
 		var playback_item = new MenuItem (null, null);
-		playback_item.set_attribute ("x-canonical-type", "s", "com.canonical.unity.playback-item");
+		playback_item.set_attribute ("x-canonical-type", "s", PLAYBACK_ITEM_TYPE);
 		if (player.is_running) {
 			if (player.can_do_play) {
 				playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
@@ -345,9 +336,9 @@ public class SoundMenu: Object
 			player_item.set_attribute_value ("icon", icon.serialize ());
 		section.append_item (player_item);
 
-		var playback_item = create_playback_menu_item (player);
 		if (player.is_running|| !this.hide_inactive_player_controls) {
-			section.insert_item (PlayerSectionPosistion.PLAYER_CONTROLS, playback_item);
+			var playback_item = create_playback_menu_item (player);
+			section.insert_item (PlayerSectionPosition.PLAYER_CONTROLS, playback_item);
 		}
 
 		/* Add new players to the end of the player sections, just before the settings */
@@ -374,14 +365,14 @@ public class SoundMenu: Object
 		if (player.is_running || !this.hide_inactive_player_controls) {
 			MenuItem playback_item = create_playback_menu_item (player);
 			if (play_control_index != -1) {
-				player_section.remove (PlayerSectionPosistion.PLAYER_CONTROLS);	
+				player_section.remove (PlayerSectionPosition.PLAYER_CONTROLS);	
 			}
-			player_section.insert_item (PlayerSectionPosistion.PLAYER_CONTROLS, playback_item);
+			player_section.insert_item (PlayerSectionPosition.PLAYER_CONTROLS, playback_item);
 		} else {
 			if (play_control_index != -1 && number_of_running_players >= 1) {
 				// remove both, playlist and play controls
-				player_section.remove (PlayerSectionPosistion.PLAYLIST);
-				player_section.remove (PlayerSectionPosistion.PLAYER_CONTROLS);	
+				player_section.remove (PlayerSectionPosition.PLAYLIST);
+				player_section.remove (PlayerSectionPosition.PLAYER_CONTROLS);	
 			}
 		}
 	}
diff --git a/tests/sound-menu.cc b/tests/sound-menu.cc
index 9f0181c..895b723 100644
--- a/tests/sound-menu.cc
+++ b/tests/sound-menu.cc
@@ -96,9 +96,22 @@ class SoundMenuTest : public ::testing::Test
 
             /* Player control */
             verify_item_attribute(section, 1, "x-canonical-type", g_variant_new_string("com.canonical.unity.playback-item"));
-            verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string(canPlay ? "indicator.play.player-id" : "indicator.play.player-id.disabled"));
-            verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string(canNext ? "indicator.next.player-id" : "indicator.next.player-id.disabled"));
-            verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string(canPrev ? "indicator.previous.player-id" : "indicator.previous.player-id.disabled"));
+	    //verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string(""));
+	    if (!canPlay) {
+                verify_item_attribute_is_not_set(section, 1, "x-canonical-play-action", G_VARIANT_TYPE_STRING);
+	    } else {
+	        verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string("indicator.play.player-id"));
+	    }
+	    if (!canNext) {
+                verify_item_attribute_is_not_set(section, 1, "x-canonical-next-action", G_VARIANT_TYPE_STRING);
+	    } else {
+	        verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string("indicator.next.player-id"));
+	    }
+	    if (!canPrev) {
+                verify_item_attribute_is_not_set(section, 1, "x-canonical-previous-action", G_VARIANT_TYPE_STRING);
+	    } else {
+	        verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string("indicator.previous.player-id"));
+	    }
 
             g_clear_object(&section);
 
@@ -170,4 +183,21 @@ TEST_F(SoundMenuTest, AddRemovePlayer) {
     return;
 }
 
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPlayNextPrev) {
+	check_player_control_buttons(false, false, false);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoNext) {
+	check_player_control_buttons(true, false, true);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPrev) {
+	check_player_control_buttons(true, true, false);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPlay) {
+	check_player_control_buttons(false, true, true);
+}
+
 //