Reduce jank when revoking notification permission
-Reorder fields so the appear/disappear more cleanly
-Remove some divider lines- they don't animate with the content, for
some reason
-Remove an unused field
-Hide the badge field (remaining field with divider) if there are no
channels
-Change style of 'Notifications are blocked' field to better match
the other fields
Test: revoke notification permission for apps with and without channels
Test: BadgePreferenceControllerTest
Fixes: 233962859
Change-Id: Ife894172bf91d838edf3b0546c5ee00e206a855f
diff --git a/res/xml/app_notification_settings.xml b/res/xml/app_notification_settings.xml
index 5b744fb..9857a4d 100644
--- a/res/xml/app_notification_settings.xml
+++ b/res/xml/app_notification_settings.xml
@@ -26,17 +26,15 @@
<com.android.settings.widget.SettingsMainSwitchPreference
android:key="block" />
- <com.android.settingslib.widget.FooterPreference
+ <Preference
android:key="block_desc" />
-
<!-- Conversations added here -->
<PreferenceCategory
android:title="@string/conversations_category_title"
android:key="conversations"
- android:visibility="gone"
- settings:allowDividerAbove="false"
- settings:allowDividerBelow="false">
+ android:visibility="gone">
+
</PreferenceCategory>
<com.android.settingslib.RestrictedSwitchPreference
android:key="invalid_conversation_switch"
@@ -49,25 +47,13 @@
android:key="bubble_pref_link"
android:title="@string/notification_bubbles_title"
android:icon="@drawable/ic_create_bubble"
- settings:allowDividerAbove="false"
settings:controller="com.android.settings.notification.app.BubbleSummaryPreferenceController">
</Preference>
<!-- Channels/Channel groups added here -->
<PreferenceCategory
android:key="channels"
- android:layout="@layout/empty_view"
- settings:allowDividerAbove="true"
- settings:allowDividerBelow="true" />
-
- <!-- Show badge -->
- <com.android.settingslib.RestrictedSwitchPreference
- android:key="badge"
- android:title="@string/notification_badge_title"
- settings:useAdditionalSummary="true"
- android:order="1001"
- settings:allowDividerAbove="true"
- settings:restrictedSwitchSummary="@string/enabled_by_admin" />
+ android:layout="@layout/empty_view" />
<!-- Importance toggle -->
<com.android.settingslib.RestrictedSwitchPreference
@@ -86,18 +72,25 @@
android:title="@string/app_notification_override_dnd_title"
android:summary="@string/app_notification_override_dnd_summary"/>
- <Preference
- android:key="app_link"
- android:order="1003"
- android:icon="@drawable/ic_settings_24dp"
- android:title="@string/app_settings_link" />
+ <!-- Show badge -->
+ <com.android.settingslib.RestrictedSwitchPreference
+ android:key="badge"
+ android:title="@string/notification_badge_title"
+ settings:useAdditionalSummary="true"
+ android:order="1001"
+ android:icon="@drawable/ic_notification_dot"
+ settings:allowDividerAbove="true"
+ settings:restrictedSwitchSummary="@string/enabled_by_admin" />
- <com.android.settingslib.widget.FooterPreference
- android:key="desc"
- android:order="5000" />
+ <Preference
+ android:key="app_link"
+ android:order="1003"
+ android:icon="@drawable/ic_settings_24dp"
+ android:title="@string/app_settings_link" />
<com.android.settingslib.widget.FooterPreference
android:key="deleted"
+ android:icon="@drawable/ic_trash_can"
android:order="8000" />
</PreferenceScreen>
diff --git a/src/com/android/settings/notification/app/AppNotificationSettings.java b/src/com/android/settings/notification/app/AppNotificationSettings.java
index ebcd261..5a14bc9 100644
--- a/src/com/android/settings/notification/app/AppNotificationSettings.java
+++ b/src/com/android/settings/notification/app/AppNotificationSettings.java
@@ -98,14 +98,13 @@
mBackend));
mControllers.add(new DndPreferenceController(context, mBackend));
mControllers.add(new AppLinkPreferenceController(context));
- mControllers.add(new DescriptionPreferenceController(context));
- mControllers.add(new NotificationsOffPreferenceController(context));
- mControllers.add(new DeletedChannelsPreferenceController(context, mBackend));
mControllers.add(new ChannelListPreferenceController(context, mBackend));
mControllers.add(new AppConversationListPreferenceController(context, mBackend));
mControllers.add(new InvalidConversationInfoPreferenceController(context, mBackend));
mControllers.add(new InvalidConversationPreferenceController(context, mBackend));
mControllers.add(new BubbleSummaryPreferenceController(context, mBackend));
+ mControllers.add(new NotificationsOffPreferenceController(context));
+ mControllers.add(new DeletedChannelsPreferenceController(context, mBackend));
return new ArrayList<>(mControllers);
}
}
diff --git a/src/com/android/settings/notification/app/BadgePreferenceController.java b/src/com/android/settings/notification/app/BadgePreferenceController.java
index 108fa1d..9d55fa3 100644
--- a/src/com/android/settings/notification/app/BadgePreferenceController.java
+++ b/src/com/android/settings/notification/app/BadgePreferenceController.java
@@ -62,9 +62,16 @@
if (isDefaultChannel()) {
return true;
} else {
- return mAppRow == null ? false : mAppRow.showBadge;
+ return mAppRow == null
+ ? false
+ : mAppRow.channelCount == 0
+ ? false
+ : mAppRow.showBadge;
}
}
+ if (mAppRow.channelCount == 0) {
+ return false;
+ }
return true;
}
diff --git a/src/com/android/settings/notification/app/DescriptionPreferenceController.java b/src/com/android/settings/notification/app/DescriptionPreferenceController.java
deleted file mode 100644
index 0a5bb2f..0000000
--- a/src/com/android/settings/notification/app/DescriptionPreferenceController.java
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.settings.notification.app;
-
-import android.content.Context;
-import android.text.TextUtils;
-
-import androidx.preference.Preference;
-
-import com.android.settings.core.PreferenceControllerMixin;
-
-public class DescriptionPreferenceController extends NotificationPreferenceController
- implements PreferenceControllerMixin {
-
- private static final String KEY_DESC = "desc";
-
- public DescriptionPreferenceController(Context context) {
- super(context, null);
- }
-
- @Override
- public String getPreferenceKey() {
- return KEY_DESC;
- }
-
- @Override
- public boolean isAvailable() {
- if (!super.isAvailable()) {
- return false;
- }
- if (mChannel == null && !hasValidGroup()) {
- return false;
- }
- if (mChannel != null && !TextUtils.isEmpty(mChannel.getDescription())) {
- return true;
- }
- if (hasValidGroup() && !TextUtils.isEmpty(mChannelGroup.getDescription())) {
- return true;
- }
- return false;
- }
-
- @Override
- boolean isIncludedInFilter() {
- return false;
- }
-
- public void updateState(Preference preference) {
- if (mAppRow != null) {
- if (mChannel != null) {
- preference.setTitle(mChannel.getDescription());
- } else if (hasValidGroup()) {
- preference.setTitle(mChannelGroup.getDescription());
- }
- }
- preference.setEnabled(false);
- preference.setSelectable(false);
- }
-}
diff --git a/tests/robotests/src/com/android/settings/notification/app/BadgePreferenceControllerTest.java b/tests/robotests/src/com/android/settings/notification/app/BadgePreferenceControllerTest.java
index d871776..cf852c1 100644
--- a/tests/robotests/src/com/android/settings/notification/app/BadgePreferenceControllerTest.java
+++ b/tests/robotests/src/com/android/settings/notification/app/BadgePreferenceControllerTest.java
@@ -97,6 +97,7 @@
@Test
public void testIsAvailable_notIfAppBlocked() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
appRow.banned = true;
mController.onResume(appRow, mock(NotificationChannel.class), null, null, null, null, null);
assertFalse(mController.isAvailable());
@@ -105,6 +106,7 @@
@Test
public void testIsAvailable_notIfChannelBlocked() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_NONE);
mController.onResume(appRow, channel, null, null, null, null, null);
@@ -114,6 +116,7 @@
@Test
public void testIsAvailable_channel_notIfAppOff() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
appRow.showBadge = false;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
@@ -125,6 +128,7 @@
@Test
public void testIsAvailable_notIfOffGlobally() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
mController.onResume(appRow, channel, null, null, null, null, null);
@@ -136,6 +140,7 @@
@Test
public void testIsAvailable_app() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
mController.onResume(appRow, null, null, null, null, null, null);
Settings.Secure.putInt(mContext.getContentResolver(), NOTIFICATION_BADGING, 1);
@@ -143,9 +148,20 @@
}
@Test
+ public void testIsAvailable_appNoChannels() {
+ NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 0;
+ mController.onResume(appRow, null, null, null, null, null, null);
+ Settings.Secure.putInt(mContext.getContentResolver(), NOTIFICATION_BADGING, 1);
+
+ assertFalse(mController.isAvailable());
+ }
+
+ @Test
public void testIsAvailable_defaultChannel() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
when(channel.getId()).thenReturn(DEFAULT_CHANNEL_ID);
@@ -159,6 +175,7 @@
public void testIsAvailable_channel() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
mController.onResume(appRow, channel, null, null, null, null, null);
@@ -183,6 +200,7 @@
public void testIsAvailable_filteredOut() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
mController.onResume(appRow, channel, null, null, null, null, new ArrayList<>());
@@ -195,6 +213,7 @@
public void testIsAvailable_filteredIn() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getImportance()).thenReturn(IMPORTANCE_HIGH);
mController.onResume(appRow, channel, null, null, null, null,
@@ -206,9 +225,11 @@
@Test
public void testUpdateState_disabledByAdmin() {
+ NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.getId()).thenReturn("something");
- mController.onResume(new NotificationBackend.AppRow(), channel, null,
+ mController.onResume(appRow, channel, null,
null, null, mock(RestrictedLockUtils.EnforcedAdmin.class), null);
Preference pref = new RestrictedSwitchPreference(mContext);
@@ -220,6 +241,7 @@
@Test
public void testUpdateState_channel() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
+ appRow.channelCount = 1;
NotificationChannel channel = mock(NotificationChannel.class);
when(channel.canShowBadge()).thenReturn(true);
mController.onResume(appRow, channel, null, null, null, null, null);
@@ -240,6 +262,7 @@
public void testUpdateState_app() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
mController.onResume(appRow, null, null, null, null, null, null);
RestrictedSwitchPreference pref = new RestrictedSwitchPreference(mContext);
@@ -257,6 +280,7 @@
public void testOnPreferenceChange_on_channel() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel =
new NotificationChannel(DEFAULT_CHANNEL_ID, "a", IMPORTANCE_LOW);
channel.setShowBadge(false);
@@ -276,6 +300,7 @@
public void testOnPreferenceChange_off_channel() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
NotificationChannel channel =
new NotificationChannel(DEFAULT_CHANNEL_ID, "a", IMPORTANCE_HIGH);
channel.setShowBadge(true);
@@ -295,6 +320,7 @@
public void testOnPreferenceChange_on_app() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = false;
+ appRow.channelCount = 1;
mController.onResume(appRow, null, null, null, null, null, null);
RestrictedSwitchPreference pref = new RestrictedSwitchPreference(mContext);
@@ -312,6 +338,7 @@
public void testOnPreferenceChange_off_app() {
NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
appRow.showBadge = true;
+ appRow.channelCount = 1;
mController.onResume(appRow, null, null, null, null, null, null);
RestrictedSwitchPreference pref = new RestrictedSwitchPreference(mContext);
diff --git a/tests/robotests/src/com/android/settings/notification/app/DescriptionPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/notification/app/DescriptionPreferenceControllerTest.java
deleted file mode 100644
index 3ce67a2..0000000
--- a/tests/robotests/src/com/android/settings/notification/app/DescriptionPreferenceControllerTest.java
+++ /dev/null
@@ -1,170 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.settings.notification.app;
-
-import static android.app.NotificationManager.IMPORTANCE_LOW;
-import static android.app.NotificationManager.IMPORTANCE_NONE;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.when;
-
-import android.app.NotificationChannel;
-import android.app.NotificationChannelGroup;
-import android.app.NotificationManager;
-import android.content.Context;
-import android.os.UserManager;
-
-import androidx.preference.Preference;
-
-import com.android.settings.notification.NotificationBackend;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
-import org.robolectric.RobolectricTestRunner;
-import org.robolectric.RuntimeEnvironment;
-import org.robolectric.shadows.ShadowApplication;
-
-@RunWith(RobolectricTestRunner.class)
-public class DescriptionPreferenceControllerTest {
-
- private Context mContext;
- @Mock
- private NotificationManager mNm;
- @Mock
- private UserManager mUm;
-
- private DescriptionPreferenceController mController;
-
- @Before
- public void setUp() {
- MockitoAnnotations.initMocks(this);
- ShadowApplication shadowApplication = ShadowApplication.getInstance();
- shadowApplication.setSystemService(Context.NOTIFICATION_SERVICE, mNm);
- shadowApplication.setSystemService(Context.USER_SERVICE, mUm);
- mContext = RuntimeEnvironment.application;
- mController = spy(new DescriptionPreferenceController(mContext));
- }
-
- @Test
- public void testNoCrashIfNoOnResume() {
- mController.isAvailable();
- mController.updateState(mock(Preference.class));
- }
-
- @Test
- public void testIsAvailable_notIfNull() {
- mController.onResume(null, null, null, null, null, null, null);
- assertFalse(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_notIfChannelGroupBlocked() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannelGroup group = mock(NotificationChannelGroup.class);
- mController.onResume(appRow, null, group, null, null, null, null);
- assertFalse(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_notIfChannelBlocked() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannel channel = mock(NotificationChannel.class);
- when(channel.getImportance()).thenReturn(IMPORTANCE_NONE);
- mController.onResume(appRow, channel, null, null, null, null, null);
- assertFalse(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_notIfNoChannelDesc() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannel channel = mock(NotificationChannel.class);
- when(channel.getImportance()).thenReturn(IMPORTANCE_LOW);
- mController.onResume(appRow, channel, null, null, null, null, null);
- assertFalse(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_notIfNoChannelGroupDesc() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannelGroup group = mock(NotificationChannelGroup.class);
- mController.onResume(appRow, null, group, null, null, null, null);
- assertFalse(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_channel() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannel channel = mock(NotificationChannel.class);
- when(channel.getImportance()).thenReturn(IMPORTANCE_LOW);
- when(channel.getDescription()).thenReturn("AAA");
- mController.onResume(appRow, channel, null, null, null, null, null);
- assertTrue(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_channelGroup() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannelGroup group = mock(NotificationChannelGroup.class);
- when(group.getDescription()).thenReturn("something");
- when(group.isBlocked()).thenReturn(false);
- mController.onResume(appRow, null, group, null, null, null, null);
- assertTrue(mController.isAvailable());
- }
-
- @Test
- public void testIsAvailable_alwaysFiltered() {
- assertFalse(mController.isIncludedInFilter());
- }
-
- @Test
- public void testUpdateState_channel() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannel channel = mock(NotificationChannel.class);
- when(channel.getImportance()).thenReturn(IMPORTANCE_LOW);
- when(channel.getDescription()).thenReturn("AAA");
- mController.onResume(appRow, channel, null, null, null, null, null);
-
- Preference pref = new Preference(RuntimeEnvironment.application);
- mController.updateState(pref);
-
- assertEquals("AAA", pref.getTitle());
- assertFalse(pref.isEnabled());
- assertFalse(pref.isSelectable());
- }
-
- @Test
- public void testUpdateState_channelGroup() {
- NotificationBackend.AppRow appRow = new NotificationBackend.AppRow();
- NotificationChannelGroup group = mock(NotificationChannelGroup.class);
- when(group.getDescription()).thenReturn("something");
- mController.onResume(appRow, null, group, null, null, null, null);
-
- Preference pref = new Preference(RuntimeEnvironment.application);
- mController.updateState(pref);
-
- assertEquals("something", pref.getTitle());
- assertFalse(pref.isEnabled());
- assertFalse(pref.isSelectable());
- }
-}