From f04c1674757e5bf678d5503309d6da4580f68f94 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 15 Jul 2020 16:45:54 -0400 Subject: [PATCH] Fix overlapping importance icons We can't re-use the same background because not all of the buttons are the same size Test: manual Bug: 161297551 Change-Id: I8583cb2fbbcb971ab5819eefd84dde3f7c3b4bdf --- ...notif_priority_conversation_preference.xml | 30 ++--- .../app/ConversationPriorityPreference.java | 111 +++++------------- .../ConversationPriorityPreferenceTest.java | 55 +-------- 3 files changed, 49 insertions(+), 147 deletions(-) diff --git a/res/layout/notif_priority_conversation_preference.xml b/res/layout/notif_priority_conversation_preference.xml index f68dbdef8ff..9c1a302304f 100644 --- a/res/layout/notif_priority_conversation_preference.xml +++ b/res/layout/notif_priority_conversation_preference.xml @@ -32,7 +32,7 @@ android:clickable="true" android:focusable="true"> @@ -76,7 +76,7 @@ android:clickable="true" android:focusable="true"> @@ -120,7 +120,7 @@ android:clickable="true" android:focusable="true"> diff --git a/src/com/android/settings/notification/app/ConversationPriorityPreference.java b/src/com/android/settings/notification/app/ConversationPriorityPreference.java index 307abec97b1..67bffbf64b2 100644 --- a/src/com/android/settings/notification/app/ConversationPriorityPreference.java +++ b/src/com/android/settings/notification/app/ConversationPriorityPreference.java @@ -50,8 +50,6 @@ public class ConversationPriorityPreference extends Preference { private View mAlertButton; private View mPriorityButton; private Context mContext; - Drawable selectedBackground; - Drawable unselectedBackground; private static final int BUTTON_ANIM_TIME_MS = 100; public ConversationPriorityPreference(Context context, AttributeSet attrs, @@ -77,8 +75,6 @@ public class ConversationPriorityPreference extends Preference { private void init(Context context) { mContext = context; - selectedBackground = mContext.getDrawable(R.drawable.button_border_selected); - unselectedBackground = mContext.getDrawable(R.drawable.button_border_unselected); setLayoutResource(R.layout.notif_priority_conversation_preference); } @@ -148,86 +144,43 @@ public class ConversationPriorityPreference extends Preference { TransitionManager.beginDelayedTransition(parent, transition); } - ColorStateList colorAccent = getAccentTint(); - ColorStateList colorNormal = getRegularTint(); - ImageView silenceIcon = parent.findViewById(R.id.silence_icon); - TextView silenceLabel = parent.findViewById(R.id.silence_label); - TextView silenceSummary = parent.findViewById(R.id.silence_summary); - ImageView alertIcon = parent.findViewById(R.id.alert_icon); - TextView alertLabel = parent.findViewById(R.id.alert_label); - TextView alertSummary = parent.findViewById(R.id.alert_summary); - ImageView priorityIcon = parent.findViewById(R.id.priority_icon); - TextView priorityLabel = parent.findViewById(R.id.priority_label); - TextView prioritySummary = parent.findViewById(R.id.priority_summary); - if (importance <= IMPORTANCE_LOW && importance > IMPORTANCE_UNSPECIFIED) { - alertSummary.setVisibility(GONE); - alertIcon.setImageTintList(colorNormal); - alertLabel.setTextColor(colorNormal); - - prioritySummary.setVisibility(GONE); - priorityIcon.setImageTintList(colorNormal); - priorityLabel.setTextColor(colorNormal); - - silenceIcon.setImageTintList(colorAccent); - silenceLabel.setTextColor(colorAccent); - silenceSummary.setVisibility(VISIBLE); - - mAlertButton.setBackground(unselectedBackground); - mPriorityButton.setBackground(unselectedBackground); - mSilenceButton.setBackground(selectedBackground); - // a11y service won't always read the newly appearing text in the right order if the - // selection happens too soon (readback happens on a different thread as layout). post - // the selection to make that conflict less likely - parent.post(() -> { - mSilenceButton.setSelected(true); - mAlertButton.setSelected(false); - mPriorityButton.setSelected(false); - }); + setSelected(mPriorityButton, false); + setSelected(mAlertButton, false); + setSelected(mSilenceButton, true); } else { if (isPriority) { - alertSummary.setVisibility(GONE); - alertIcon.setImageTintList(colorNormal); - alertLabel.setTextColor(colorNormal); - - prioritySummary.setVisibility(VISIBLE); - priorityIcon.setImageTintList(colorAccent); - priorityLabel.setTextColor(colorAccent); - - silenceIcon.setImageTintList(colorNormal); - silenceLabel.setTextColor(colorNormal); - silenceSummary.setVisibility(GONE); - - mAlertButton.setBackground(unselectedBackground); - mPriorityButton.setBackground(selectedBackground); - mSilenceButton.setBackground(unselectedBackground); - parent.post(() -> { - mSilenceButton.setSelected(false); - mAlertButton.setSelected(false); - mPriorityButton.setSelected(true); - }); + setSelected(mPriorityButton, true); + setSelected(mAlertButton, false); + setSelected(mSilenceButton, false); } else { - alertSummary.setVisibility(VISIBLE); - alertIcon.setImageTintList(colorAccent); - alertLabel.setTextColor(colorAccent); - - prioritySummary.setVisibility(GONE); - priorityIcon.setImageTintList(colorNormal); - priorityLabel.setTextColor(colorNormal); - - silenceIcon.setImageTintList(colorNormal); - silenceLabel.setTextColor(colorNormal); - silenceSummary.setVisibility(GONE); - - mAlertButton.setBackground(selectedBackground); - mPriorityButton.setBackground(unselectedBackground); - mSilenceButton.setBackground(unselectedBackground); - parent.post(() -> { - mSilenceButton.setSelected(false); - mAlertButton.setSelected(true); - mPriorityButton.setSelected(false); - }); + setSelected(mPriorityButton, false); + setSelected(mAlertButton, true); + setSelected(mSilenceButton, false); } } } + + void setSelected(View view, boolean selected) { + ColorStateList colorAccent = getAccentTint(); + ColorStateList colorNormal = getRegularTint(); + + ImageView icon = view.findViewById(R.id.icon); + TextView label = view.findViewById(R.id.label); + TextView summary = view.findViewById(R.id.summary); + + icon.setImageTintList(selected ? colorAccent : colorNormal); + label.setTextColor(selected ? colorAccent : colorNormal); + summary.setVisibility(selected ? VISIBLE : GONE); + + view.setBackground(mContext.getDrawable(selected + ? R.drawable.button_border_selected + : R.drawable.button_border_unselected)); + // a11y service won't always read the newly appearing text in the right order if the + // selection happens too soon (readback happens on a different thread as layout). post + // the selection to make that conflict less likely + view.post(() -> { + view.setSelected(selected); + }); + } } diff --git a/tests/robotests/src/com/android/settings/notification/app/ConversationPriorityPreferenceTest.java b/tests/robotests/src/com/android/settings/notification/app/ConversationPriorityPreferenceTest.java index 12e1f35c767..2d2fcc8261b 100644 --- a/tests/robotests/src/com/android/settings/notification/app/ConversationPriorityPreferenceTest.java +++ b/tests/robotests/src/com/android/settings/notification/app/ConversationPriorityPreferenceTest.java @@ -68,10 +68,6 @@ public class ConversationPriorityPreferenceTest { final LayoutInflater inflater = LayoutInflater.from(mContext); PreferenceViewHolder holder = PreferenceViewHolder.createInstanceForTests( inflater.inflate(preference.getLayoutResource(), null)); - Drawable unselected = mock(Drawable.class); - Drawable selected = mock(Drawable.class); - preference.selectedBackground = selected; - preference.unselectedBackground = unselected; preference.setConfigurable(false); preference.setImportance(IMPORTANCE_DEFAULT); @@ -81,35 +77,6 @@ public class ConversationPriorityPreferenceTest { assertThat(holder.itemView.findViewById(R.id.silence).isEnabled()).isFalse(); assertThat(holder.itemView.findViewById(R.id.priority_group).isEnabled()).isFalse(); assertThat(holder.itemView.findViewById(R.id.alert).isEnabled()).isFalse(); - - assertThat(holder.itemView.findViewById(R.id.priority_group).getBackground()) - .isEqualTo(selected); - assertThat(holder.itemView.findViewById(R.id.alert).getBackground()).isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.silence).getBackground()) - .isEqualTo(unselected); - - // other button - preference.setPriorityConversation(false); - holder = PreferenceViewHolder.createInstanceForTests( - inflater.inflate(preference.getLayoutResource(), null)); - preference.onBindViewHolder(holder); - - assertThat(holder.itemView.findViewById(R.id.alert).getBackground()).isEqualTo(selected); - assertThat(holder.itemView.findViewById(R.id.silence).getBackground()) - .isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.priority_group).getBackground()) - .isEqualTo(unselected); - - // other other button - preference.setImportance(IMPORTANCE_LOW); - holder = PreferenceViewHolder.createInstanceForTests( - inflater.inflate(preference.getLayoutResource(), null)); - preference.onBindViewHolder(holder); - - assertThat(holder.itemView.findViewById(R.id.priority_group).getBackground()) - .isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.alert).getBackground()).isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.silence).getBackground()).isEqualTo(selected); } @Test @@ -119,10 +86,6 @@ public class ConversationPriorityPreferenceTest { final LayoutInflater inflater = LayoutInflater.from(mContext); final PreferenceViewHolder holder = PreferenceViewHolder.createInstanceForTests( inflater.inflate(preference.getLayoutResource(), null)); - Drawable unselected = mock(Drawable.class); - Drawable selected = mock(Drawable.class); - preference.selectedBackground = selected; - preference.unselectedBackground = unselected; preference.setConfigurable(true); preference.setImportance(IMPORTANCE_LOW); @@ -130,12 +93,8 @@ public class ConversationPriorityPreferenceTest { preference.onBindViewHolder(holder); - assertThat(holder.itemView.findViewById(R.id.priority_group).getBackground()) - .isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.alert).getBackground()).isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.silence).getBackground()) - .isEqualTo(selected); - assertThat(holder.itemView.findViewById(R.id.silence_summary).getVisibility()) + assertThat(holder.itemView.findViewById(R.id.silence) + .findViewById(R.id.summary).getVisibility()) .isEqualTo(View.VISIBLE); } @@ -146,10 +105,6 @@ public class ConversationPriorityPreferenceTest { final LayoutInflater inflater = LayoutInflater.from(mContext); final PreferenceViewHolder holder = PreferenceViewHolder.createInstanceForTests( inflater.inflate(preference.getLayoutResource(), null)); - Drawable unselected = mock(Drawable.class); - Drawable selected = mock(Drawable.class); - preference.selectedBackground = selected; - preference.unselectedBackground = unselected; preference.setConfigurable(true); preference.setImportance(IMPORTANCE_DEFAULT); @@ -161,12 +116,6 @@ public class ConversationPriorityPreferenceTest { silenceButton.callOnClick(); - assertThat(holder.itemView.findViewById(R.id.alert).getBackground()).isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.priority_group).getBackground()) - .isEqualTo(unselected); - assertThat(holder.itemView.findViewById(R.id.silence).getBackground()) - .isEqualTo(selected); - verify(preference, times(1)).callChangeListener(new Pair(IMPORTANCE_LOW, false)); } }