Improve notification UX.

- Add a proper notification icon (courtesy of Joey Rizzoli)
- Use separate notification modes to allow the user to dismiss the
  notification when music isn't playing.
- Always show notification regardless whether an activity is in
  foreground to ensure the notification doesn't pop up out of the blue
  when ending playback inside the app and leaving it afterwards.
- Keep notification timestamp constant when updating it

Change-Id: I2fa9a56ff31ab2874d6d96786e6b80695397702b

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index 37122a6..3db8502 100644 (file)
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -19,10 +19,9 @@
     android:versionCode="2"
     android:versionName="1.1" >
 
-    <!-- ICS to Jelly Bean -->
     <uses-sdk
         android:minSdkVersion="17"
-        android:targetSdkVersion="20" />
+        android:targetSdkVersion="21" />
 
     <!-- Used for caching and creating new playlists -->
     <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />

diff --git a/res/drawable-hdpi/ic_notification.png b/res/drawable-hdpi/ic_notification.png
new file mode 100644 (file)
index 0000000..5610f64
Binary files /dev/null and b/res/drawable-hdpi/ic_notification.png differ

diff --git a/res/drawable-mdpi/ic_notification.png b/res/drawable-mdpi/ic_notification.png
new file mode 100644 (file)
index 0000000..9511ea6
Binary files /dev/null and b/res/drawable-mdpi/ic_notification.png differ

diff --git a/res/drawable-xhdpi/ic_notification.png b/res/drawable-xhdpi/ic_notification.png
new file mode 100644 (file)
index 0000000..564c5d0
Binary files /dev/null and b/res/drawable-xhdpi/ic_notification.png differ

diff --git a/res/drawable-xxhdpi/ic_notification.png b/res/drawable-xxhdpi/ic_notification.png
new file mode 100644 (file)
index 0000000..1203bc6
Binary files /dev/null and b/res/drawable-xxhdpi/ic_notification.png differ

diff --git a/src/com/cyanogenmod/eleven/MusicPlaybackService.java b/src/com/cyanogenmod/eleven/MusicPlaybackService.java
index 0a598fe..c61c9f6 100644 (file)
--- a/src/com/cyanogenmod/eleven/MusicPlaybackService.java
+++ b/src/com/cyanogenmod/eleven/MusicPlaybackService.java
@@ -16,6 +16,7 @@ package com.cyanogenmod.eleven;
 import android.annotation.SuppressLint;
 import android.app.AlarmManager;
 import android.app.Notification;
+import android.app.NotificationManager;
 import android.app.PendingIntent;
 import android.app.Service;
 import android.appwidget.AppWidgetManager;
@@ -181,13 +182,6 @@ public class MusicPlaybackService extends Service {
      */
     public static final String SHUFFLE_ACTION = "com.cyanogenmod.eleven.shuffle";
 
-    /**
-     * Called to update the service about the foreground state of Apollo's activities
-     */
-    public static final String FOREGROUND_STATE_CHANGED = "com.cyanogenmod.eleven.fgstatechanged";
-
-    public static final String NOW_IN_FOREGROUND = "nowinforeground";
-
     public static final String FROM_MEDIA_BUTTON = "frommediabutton";
 
     /**
@@ -408,6 +402,8 @@ public class MusicPlaybackService extends Service {
     private PendingIntent mShutdownIntent;
     private boolean mShutdownScheduled;
 
+    private NotificationManager mNotificationManager;
+
     /**
      * The cursor used to retrieve info on the current track and run the
      * necessary queries to play audio files
@@ -445,6 +441,13 @@ public class MusicPlaybackService extends Service {
      */
     private long mLastPlayedTime;
 
+    private int mNotifyMode = NOTIFY_MODE_NONE;
+    private long mNotificationPostTime = 0;
+
+    private static final int NOTIFY_MODE_NONE = 0;
+    private static final int NOTIFY_MODE_FOREGROUND = 1;
+    private static final int NOTIFY_MODE_BACKGROUND = 2;
+
     /**
      * Used to indicate if the queue can be saved
      */
@@ -456,11 +459,6 @@ public class MusicPlaybackService extends Service {
     private boolean mPausedByTransientLossOfFocus = false;
 
     /**
-     * Used to track whether any of Apollo's activities is in the foreground
-     */
-    private boolean mAnyActivityInForeground = false;
-
-    /**
      * Lock screen controls
      */
     private MediaSession mSession;
@@ -576,6 +574,8 @@ public class MusicPlaybackService extends Service {
         if (D) Log.d(TAG, "Creating service");
         super.onCreate();
 
+        mNotificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
+
         // Initialize the favorites and recents databases
         mRecentsCache = RecentStore.getInstance(this);
 
@@ -696,7 +696,6 @@ public class MusicPlaybackService extends Service {
             }
         });
         mSession.setFlags(MediaSession.FLAG_HANDLES_TRANSPORT_CONTROLS);
-        mSession.setActive(true);
     }
 
     /**
@@ -757,11 +756,6 @@ public class MusicPlaybackService extends Service {
         if (intent != null) {
             final String action = intent.getAction();
 
-            if (intent.hasExtra(NOW_IN_FOREGROUND)) {
-                mAnyActivityInForeground = intent.getBooleanExtra(NOW_IN_FOREGROUND, false);
-                updateNotification();
-            }
-
             if (SHUTDOWN.equals(action)) {
                 mShutdownScheduled = false;
                 releaseServiceUiAndStop();
@@ -790,8 +784,9 @@ public class MusicPlaybackService extends Service {
         }
 
         if (D) Log.d(TAG, "Nothing is playing anymore, releasing notification");
-        stopForeground(true);
+        cancelNotification();
         mAudioManager.abandonAudioFocus(mAudioFocusListener);
+        mSession.setActive(false);
 
         if (!mServiceInUse) {
             saveQueue(true);
@@ -838,11 +833,39 @@ public class MusicPlaybackService extends Service {
      * Updates the notification, considering the current play and activity state
      */
     private void updateNotification() {
-        if (!mAnyActivityInForeground && recentlyPlayed()) {
-            buildNotification();
-        } else if (mAnyActivityInForeground) {
-            stopForeground(true);
+        final int newNotifyMode;
+        if (isPlaying()) {
+            newNotifyMode = NOTIFY_MODE_FOREGROUND;
+        } else if (recentlyPlayed()) {
+            newNotifyMode = NOTIFY_MODE_BACKGROUND;
+        } else {
+            newNotifyMode = NOTIFY_MODE_NONE;
+        }
+
+        int notificationId = hashCode();
+        if (mNotifyMode != newNotifyMode) {
+            if (mNotifyMode == NOTIFY_MODE_FOREGROUND) {
+                stopForeground(newNotifyMode == NOTIFY_MODE_NONE);
+            } else if (newNotifyMode == NOTIFY_MODE_NONE) {
+                mNotificationManager.cancel(notificationId);
+                mNotificationPostTime = 0;
+            }
+        }
+
+        if (newNotifyMode == NOTIFY_MODE_FOREGROUND) {
+            startForeground(notificationId, buildNotification());
+        } else if (newNotifyMode == NOTIFY_MODE_BACKGROUND) {
+            mNotificationManager.notify(notificationId, buildNotification());
         }
+
+        mNotifyMode = newNotifyMode;
+    }
+
+    private void cancelNotification() {
+        stopForeground(true);
+        mNotificationManager.cancel(hashCode());
+        mNotificationPostTime = 0;
+        mNotifyMode = NOTIFY_MODE_NONE;
     }
 
     /**
@@ -1477,7 +1500,7 @@ public class MusicPlaybackService extends Service {
         }
     }
 
-    private void buildNotification() {
+    private Notification buildNotification() {
         final String albumName = getAlbumName();
         final String artistName = getArtistName();
         final boolean isPlaying = isPlaying();
@@ -1498,16 +1521,19 @@ public class MusicPlaybackService extends Service {
         PendingIntent clickIntent = PendingIntent.getActivity(this, 0, nowPlayingIntent, 0);
         Bitmap artwork = getAlbumArt(false);
 
-        // TODO: Add back a beter small icon when we have time
+        if (mNotificationPostTime == 0) {
+            mNotificationPostTime = System.currentTimeMillis();
+        }
+
         Notification.Builder builder = new Notification.Builder(this)
-                .setSmallIcon(R.drawable.ic_launcher)
+                .setSmallIcon(R.drawable.ic_notification)
                 .setLargeIcon(artwork)
                 .setContentIntent(clickIntent)
                 .setContentTitle(getTrackName())
                 .setContentText(text)
+                .setWhen(mNotificationPostTime)
                 .setShowWhen(false)
                 .setStyle(style)
-                .setShowWhen(false)
                 .setVisibility(Notification.VISIBILITY_PUBLIC)
                 .addAction(R.drawable.btn_playback_previous,
                         getString(R.string.accessibility_prev),
@@ -1536,7 +1562,7 @@ public class MusicPlaybackService extends Service {
             builder.setColor(mCachedBitmapAccentColor);
         }
 
-        startForeground(hashCode(), builder.build());
+        return builder.build();
     }
 
     private final PendingIntent retrievePlaybackAction(final String action) {
@@ -2214,17 +2240,21 @@ public class MusicPlaybackService extends Service {
      * @param value to set mIsSupposedToBePlaying to
      * @param notify whether we want to fire PLAYSTATE_CHANGED event
      */
-    private void setIsSupposedToBePlaying(boolean value, boolean notify){
+    private void setIsSupposedToBePlaying(boolean value, boolean notify) {
         if (mIsSupposedToBePlaying != value) {
             mIsSupposedToBePlaying = value;
-            if (notify) {
-                notifyChange(PLAYSTATE_CHANGED);
-            }
 
+            // Update mLastPlayed time first and notify afterwards, as
+            // the notification listener method needs the up-to-date value
+            // for the recentlyPlayed() method to work
             if (!mIsSupposedToBePlaying) {
                 scheduleDelayedShutdown();
                 mLastPlayedTime = System.currentTimeMillis();
             }
+
+            if (notify) {
+                notifyChange(PLAYSTATE_CHANGED);
+            }
         }
     }
 
@@ -2306,6 +2336,7 @@ public class MusicPlaybackService extends Service {
 
         mAudioManager.registerMediaButtonEventReceiver(new ComponentName(getPackageName(),
                 MediaButtonIntentReceiver.class.getName()));
+        mSession.setActive(true);
 
         if (createNewNextTrack) {
             setNextTrack();

diff --git a/src/com/cyanogenmod/eleven/appwidgets/AppWidgetBase.java b/src/com/cyanogenmod/eleven/appwidgets/AppWidgetBase.java
index e72f303..24e5cfd 100644 (file)
--- a/src/com/cyanogenmod/eleven/appwidgets/AppWidgetBase.java
+++ b/src/com/cyanogenmod/eleven/appwidgets/AppWidgetBase.java
@@ -27,7 +27,6 @@ public abstract class AppWidgetBase extends AppWidgetProvider {
             final ComponentName serviceName) {
         Intent intent = new Intent(action);
         intent.setComponent(serviceName);
-        intent.putExtra(MusicPlaybackService.NOW_IN_FOREGROUND, false);
         return PendingIntent.getService(context, 0, intent, 0);
     }
 

diff --git a/src/com/cyanogenmod/eleven/ui/activities/BaseActivity.java b/src/com/cyanogenmod/eleven/ui/activities/BaseActivity.java
index da38924..bfaa343 100644 (file)
--- a/src/com/cyanogenmod/eleven/ui/activities/BaseActivity.java
+++ b/src/com/cyanogenmod/eleven/ui/activities/BaseActivity.java
@@ -232,8 +232,6 @@ public abstract class BaseActivity extends FragmentActivity implements ServiceCo
         registerReceiver(mPlaybackStatus, filter);
 
         mPlayPauseProgressButton.resume();
-
-        MusicUtils.notifyForegroundStateChanged(this, true);
     }
 
     /**
@@ -244,8 +242,6 @@ public abstract class BaseActivity extends FragmentActivity implements ServiceCo
         super.onStop();
 
         mPlayPauseProgressButton.pause();
-
-        MusicUtils.notifyForegroundStateChanged(this, false);
     }
 
     /**

diff --git a/src/com/cyanogenmod/eleven/ui/activities/SearchActivity.java b/src/com/cyanogenmod/eleven/ui/activities/SearchActivity.java
index 14594af..a538416 100644 (file)
--- a/src/com/cyanogenmod/eleven/ui/activities/SearchActivity.java
+++ b/src/com/cyanogenmod/eleven/ui/activities/SearchActivity.java
@@ -463,24 +463,6 @@ public class SearchActivity extends FragmentActivity implements
      * {@inheritDoc}
      */
     @Override
-    protected void onStart() {
-        super.onStart();
-        MusicUtils.notifyForegroundStateChanged(this, true);
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    protected void onStop() {
-        super.onStop();
-        MusicUtils.notifyForegroundStateChanged(this, false);
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
     protected void onDestroy() {
         super.onDestroy();
         // Unbind from the service

diff --git a/src/com/cyanogenmod/eleven/ui/activities/SettingsActivity.java b/src/com/cyanogenmod/eleven/ui/activities/SettingsActivity.java
index 762f222..e2ba8ee 100644 (file)
--- a/src/com/cyanogenmod/eleven/ui/activities/SettingsActivity.java
+++ b/src/com/cyanogenmod/eleven/ui/activities/SettingsActivity.java
@@ -24,7 +24,6 @@ import android.view.MenuItem;
 
 import com.cyanogenmod.eleven.R;
 import com.cyanogenmod.eleven.cache.ImageFetcher;
-import com.cyanogenmod.eleven.utils.MusicUtils;
 
 /**
  * Settings.
@@ -71,24 +70,6 @@ public class SettingsActivity extends PreferenceActivity {
     }
 
     /**
-     * {@inheritDoc}
-     */
-    @Override
-    protected void onStart() {
-        super.onStart();
-        MusicUtils.notifyForegroundStateChanged(this, true);
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    protected void onStop() {
-        super.onStop();
-        MusicUtils.notifyForegroundStateChanged(this, false);
-    }
-
-    /**
      * Removes all of the cache entries.
      */
     private void deleteCache() {

diff --git a/src/com/cyanogenmod/eleven/utils/MusicUtils.java b/src/com/cyanogenmod/eleven/utils/MusicUtils.java
index 0eb4429..441413a 100644 (file)
--- a/src/com/cyanogenmod/eleven/utils/MusicUtils.java
+++ b/src/com/cyanogenmod/eleven/utils/MusicUtils.java
@@ -76,8 +76,6 @@ public final class MusicUtils {
 
     public static IElevenService mService = null;
 
-    private static int sForegroundActivities = 0;
-
     private static final WeakHashMap<Context, ServiceBinder> mConnectionMap;
 
     private static final long[] sEmptyList;
@@ -1550,28 +1548,6 @@ public final class MusicUtils {
     }
 
     /**
-     * Used to build and show a notification when Apollo is sent into the
-     * background
-     *
-     * @param context The {@link Context} to use.
-     */
-    public static void notifyForegroundStateChanged(final Context context, boolean inForeground) {
-        int old = sForegroundActivities;
-        if (inForeground) {
-            sForegroundActivities++;
-        } else {
-            sForegroundActivities--;
-        }
-
-        if (old == 0 || sForegroundActivities == 0) {
-            final Intent intent = new Intent(context, MusicPlaybackService.class);
-            intent.setAction(MusicPlaybackService.FOREGROUND_STATE_CHANGED);
-            intent.putExtra(MusicPlaybackService.NOW_IN_FOREGROUND, sForegroundActivities != 0);
-            context.startService(intent);
-        }
-    }
-
-    /**
      * Perminately deletes item(s) from the user's device
      *
      * @param context The {@link Context} to use.