From 1f8c257d6b0f072a69fa1e0d6663e8a38ce7e0ef Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Tue, 9 Mar 2021 17:05:06 -0500 Subject: [PATCH 1/3] Fix the layout issue with the expand button of the CallStyle Bug: 179178086 Test: Post CallStyle; notice no gap where the expander would be. Change-Id: I15ca699449fa2840dba39d1c82e994d1ce2c9575 --- .../notification_template_material_call.xml | 17 +++++++++++------ .../row/NotificationContentView.java | 15 +++++++++++---- ...tificationConversationTemplateViewWrapper.kt | 7 +++++-- .../wrapper/NotificationHeaderViewWrapper.java | 10 +++++++++- .../row/wrapper/NotificationViewWrapper.java | 4 +++- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/core/res/res/layout/notification_template_material_call.xml b/core/res/res/layout/notification_template_material_call.xml index 7b52ec30abe64..c2ffd1f0d8b35 100644 --- a/core/res/res/layout/notification_template_material_call.xml +++ b/core/res/res/layout/notification_template_material_call.xml @@ -50,7 +50,6 @@ android:layout_height="wrap_content" android:layout_weight="1" android:layout_marginStart="@dimen/conversation_content_start" - android:layout_marginEnd="@dimen/notification_content_margin_end" android:orientation="vertical" android:minHeight="68dp" > @@ -71,12 +70,18 @@ /> - - + android:layout_height="match_parent" + android:minWidth="@dimen/notification_content_margin_end" + > + + + + diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java index d3065aa36a5fa..dabea582798fa 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java @@ -349,7 +349,9 @@ public class NotificationContentView extends FrameLayout { invalidateOutline(); selectLayout(false /* animate */, mForceSelectNextLayout /* force */); mForceSelectNextLayout = false; - updateExpandButtons(mExpandable); + // TODO(b/182314698): move this to onMeasure. This requires switching to getMeasuredHeight, + // and also requires revisiting all of the logic called earlier in this method. + updateExpandButtonsDuringLayout(mExpandable, true /* duringLayout */); } @Override @@ -1589,6 +1591,10 @@ public class NotificationContentView extends FrameLayout { } public void updateExpandButtons(boolean expandable) { + updateExpandButtonsDuringLayout(expandable, false /* duringLayout */); + } + + private void updateExpandButtonsDuringLayout(boolean expandable, boolean duringLayout) { mExpandable = expandable; // if the expanded child has the same height as the collapsed one we hide it. if (mExpandedChild != null && mExpandedChild.getHeight() != 0) { @@ -1602,14 +1608,15 @@ public class NotificationContentView extends FrameLayout { expandable = false; } } + boolean requestLayout = duringLayout && mIsContentExpandable != expandable; if (mExpandedChild != null) { - mExpandedWrapper.updateExpandability(expandable, mExpandClickListener); + mExpandedWrapper.updateExpandability(expandable, mExpandClickListener, requestLayout); } if (mContractedChild != null) { - mContractedWrapper.updateExpandability(expandable, mExpandClickListener); + mContractedWrapper.updateExpandability(expandable, mExpandClickListener, requestLayout); } if (mHeadsUpChild != null) { - mHeadsUpWrapper.updateExpandability(expandable, mExpandClickListener); + mHeadsUpWrapper.updateExpandability(expandable, mExpandClickListener, requestLayout); } mIsContentExpandable = expandable; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationConversationTemplateViewWrapper.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationConversationTemplateViewWrapper.kt index fb0fdcccd4b13..383bb7e41a913 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationConversationTemplateViewWrapper.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationConversationTemplateViewWrapper.kt @@ -147,8 +147,11 @@ class NotificationConversationTemplateViewWrapper constructor( override fun setRemoteInputVisible(visible: Boolean) = conversationLayout.showHistoricMessages(visible) - override fun updateExpandability(expandable: Boolean, onClickListener: View.OnClickListener?) = - conversationLayout.updateExpandability(expandable, onClickListener) + override fun updateExpandability( + expandable: Boolean, + onClickListener: View.OnClickListener, + requestLayout: Boolean + ) = conversationLayout.updateExpandability(expandable, onClickListener) override fun disallowSingleClick(x: Float, y: Float): Boolean { val isOnExpandButton = expandBtnContainer.visibility == View.VISIBLE && diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java index bdafd232167d1..5a55545351d0e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java @@ -261,7 +261,8 @@ public class NotificationHeaderViewWrapper extends NotificationViewWrapper { } @Override - public void updateExpandability(boolean expandable, View.OnClickListener onClickListener) { + public void updateExpandability(boolean expandable, View.OnClickListener onClickListener, + boolean requestLayout) { mExpandButton.setVisibility(expandable ? View.VISIBLE : View.GONE); mExpandButton.setOnClickListener(expandable ? onClickListener : null); if (mAltExpandTarget != null) { @@ -273,6 +274,13 @@ public class NotificationHeaderViewWrapper extends NotificationViewWrapper { if (mNotificationHeader != null) { mNotificationHeader.setOnClickListener(expandable ? onClickListener : null); } + // Unfortunately, the NotificationContentView has to layout its children in order to + // determine their heights, and that affects the button visibility. If that happens + // (thankfully it is rare) then we need to request layout of the expand button's parent + // in order to ensure it gets laid out correctly. + if (requestLayout) { + mExpandButton.getParent().requestLayout(); + } } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java index 9ced12d32d27a..3a7b4618ad221 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java @@ -291,8 +291,10 @@ public abstract class NotificationViewWrapper implements TransformableView { * * @param expandable should this view be expandable * @param onClickListener the listener to invoke when the expand affordance is clicked on + * @param requestLayout the expandability changed during onLayout, so a requestLayout required */ - public void updateExpandability(boolean expandable, View.OnClickListener onClickListener) {} + public void updateExpandability(boolean expandable, View.OnClickListener onClickListener, + boolean requestLayout) {} /** Set the expanded state on the view wrapper */ public void setExpanded(boolean expanded) {} From 12fa627bfb9b128a51ca7789f352805b5b61c069 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 8 Mar 2021 16:22:52 -0500 Subject: [PATCH 2/3] CallStyle now supports contextual actions (e.g. smart replies) * This required that call style also has a collapsed state which has no actions. Removing actions from the collapsed state resolves a bunch of bugs that were being introduced. There's just too much of SystemUI that assumes collapsed notifications don't have actions to fix this right now. * To make the collapsed state match stylistically, I also had to remove the progress bar from that state for vertical space reasons. Bug: 179178086 Test: atest NotificationTemplateTests Test: visual inspection ; expand/collapse Change-Id: I6bc865d5fca8459ffaa6031d220d8bfa08545d89 --- core/java/android/app/Notification.java | 24 ++-- ...otification_template_material_big_base.xml | 2 +- ...otification_template_material_big_call.xml | 107 ++++++++++++++++++ ...otification_template_material_big_text.xml | 2 +- .../notification_template_material_call.xml | 73 +++++------- core/res/res/values/symbols.xml | 1 + 6 files changed, 150 insertions(+), 59 deletions(-) create mode 100644 core/res/res/layout/notification_template_material_big_call.xml diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 2b45723dae557..c66f3f746b659 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -6583,10 +6583,6 @@ public class Notification implements Parcelable return R.layout.notification_template_material_conversation; } - private int getCallLayoutResource() { - return R.layout.notification_template_material_call; - } - private int getActionLayoutResource() { return R.layout.notification_material_action; } @@ -9329,7 +9325,7 @@ public class Notification implements Parcelable */ @Override public RemoteViews makeContentView(boolean increasedHeight) { - return makeCallLayout(); + return makeCallLayout(StandardTemplateParams.VIEW_TYPE_NORMAL); } /** @@ -9337,14 +9333,14 @@ public class Notification implements Parcelable */ @Override public RemoteViews makeHeadsUpContentView(boolean increasedHeight) { - return makeCallLayout(); + return makeCallLayout(StandardTemplateParams.VIEW_TYPE_HEADS_UP); } /** * @hide */ public RemoteViews makeBigContentView() { - return makeCallLayout(); + return makeCallLayout(StandardTemplateParams.VIEW_TYPE_BIG); } @NonNull @@ -9443,7 +9439,7 @@ public class Notification implements Parcelable return resultActions; } - private RemoteViews makeCallLayout() { + private RemoteViews makeCallLayout(int viewType) { Bundle extras = mBuilder.mN.extras; CharSequence text = mBuilder.processLegacyText(extras.getCharSequence(EXTRA_TEXT)); if (text == null) { @@ -9452,15 +9448,21 @@ public class Notification implements Parcelable // Bind standard template StandardTemplateParams p = mBuilder.mParams.reset() - .viewType(StandardTemplateParams.VIEW_TYPE_BIG) + .viewType(viewType) .callStyleActions(true) .allowTextWithProgress(true) .hideLargeIcon(true) .text(text) .summaryText(mBuilder.processLegacyText(mVerificationText)); mBuilder.mActions = getActionsListWithSystemActions(); - RemoteViews contentView = mBuilder.applyStandardTemplateWithActions( - mBuilder.getCallLayoutResource(), p, null /* result */); + final RemoteViews contentView; + if (p.mViewType != StandardTemplateParams.VIEW_TYPE_NORMAL) { + contentView = mBuilder.applyStandardTemplateWithActions( + R.layout.notification_template_material_big_call, p, null /* result */); + } else { + contentView = mBuilder.applyStandardTemplate( + R.layout.notification_template_material_call, p, null /* result */); + } // Bind some extra conversation-specific header fields. mBuilder.setTextViewColorPrimary(contentView, R.id.conversation_text, p); diff --git a/core/res/res/layout/notification_template_material_big_base.xml b/core/res/res/layout/notification_template_material_big_base.xml index 2d1c3422ca368..b9a3625f9e452 100644 --- a/core/res/res/layout/notification_template_material_big_base.xml +++ b/core/res/res/layout/notification_template_material_big_base.xml @@ -27,7 +27,7 @@ android:id="@+id/notification_action_list_margin_target" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_marginBottom="@dimen/notification_action_list_height" + android:layout_marginBottom="@dimen/notification_content_margin" android:orientation="vertical" > diff --git a/core/res/res/layout/notification_template_material_big_call.xml b/core/res/res/layout/notification_template_material_big_call.xml new file mode 100644 index 0000000000000..1d5046777e77e --- /dev/null +++ b/core/res/res/layout/notification_template_material_big_call.xml @@ -0,0 +1,107 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/res/res/layout/notification_template_material_big_text.xml b/core/res/res/layout/notification_template_material_big_text.xml index 2954ba2a09033..86e7dec29e7df 100644 --- a/core/res/res/layout/notification_template_material_big_text.xml +++ b/core/res/res/layout/notification_template_material_big_text.xml @@ -31,7 +31,7 @@ android:layout_height="wrap_content" android:layout_gravity="top" android:layout_marginTop="@dimen/notification_content_margin_top" - android:layout_marginBottom="@dimen/notification_action_list_height" + android:layout_marginBottom="@dimen/notification_content_margin" android:clipToPadding="false" android:orientation="vertical" > diff --git a/core/res/res/layout/notification_template_material_call.xml b/core/res/res/layout/notification_template_material_call.xml index c2ffd1f0d8b35..5d9e761842d84 100644 --- a/core/res/res/layout/notification_template_material_call.xml +++ b/core/res/res/layout/notification_template_material_call.xml @@ -1,5 +1,4 @@ - - + From 4018c5075acd0f40e0bb2c2349a10430ccfd25ab Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Thu, 11 Mar 2021 14:39:20 -0500 Subject: [PATCH 3/3] Hide app name from collapsed CallStyle. Also some small fixes: * Bind the title in the CallStyle builder. * Remove an unnecessary findViewById. * Rename a method overload for clarity. Bug: 179178086 Test: manual inspection Change-Id: I6e96864c17718b9729b108e4db82534aad33675f --- core/java/android/app/Notification.java | 57 ++++++++++++++----- .../android/internal/widget/CallLayout.java | 1 - .../row/NotificationContentView.java | 4 +- .../stack/NotificationChildrenContainer.java | 2 +- 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index c66f3f746b659..b31048cafbd72 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -5011,9 +5011,13 @@ public class Notification implements Parcelable boolean showProgress = handleProgressBar(contentView, ex, p); boolean hasSecondLine = showProgress; if (p.hasTitle()) { - contentView.setViewVisibility(R.id.title, View.VISIBLE); - contentView.setTextViewText(R.id.title, processTextSpans(p.title)); - setTextViewColorPrimary(contentView, R.id.title, p); + contentView.setViewVisibility(p.mTitleViewId, View.VISIBLE); + contentView.setTextViewText(p.mTitleViewId, processTextSpans(p.title)); + setTextViewColorPrimary(contentView, p.mTitleViewId, p); + } else if (p.mTitleViewId != R.id.title) { + // This alternate title view ID is not cleared by resetStandardTemplate + contentView.setViewVisibility(p.mTitleViewId, View.GONE); + contentView.setTextViewText(p.mTitleViewId, null); } if (p.text != null && p.text.length() != 0 && (!showProgress || p.mAllowTextWithProgress)) { @@ -5441,6 +5445,11 @@ public class Notification implements Parcelable // keep the divider visible between that title and the next text element. return true; } + if (p.mHideAppName) { + // The app name is being hidden, so we definitely want to return here. + // Assume that there is a title which will replace it in the header. + return p.hasTitle(); + } contentView.setViewVisibility(R.id.app_name_text, View.VISIBLE); contentView.setTextViewText(R.id.app_name_text, loadHeaderAppName()); contentView.setTextColor(R.id.app_name_text, getSecondaryTextColor(p)); @@ -5817,7 +5826,7 @@ public class Notification implements Parcelable * * @hide */ - public RemoteViews makeNotificationHeader() { + public RemoteViews makeNotificationGroupHeader() { return makeNotificationHeader(mParams.reset() .viewType(StandardTemplateParams.VIEW_TYPE_GROUP_HEADER) .fillTextsFrom(this)); @@ -9440,7 +9449,9 @@ public class Notification implements Parcelable } private RemoteViews makeCallLayout(int viewType) { + final boolean isCollapsed = viewType == StandardTemplateParams.VIEW_TYPE_NORMAL; Bundle extras = mBuilder.mN.extras; + CharSequence title = mPerson != null ? mPerson.getName() : null; CharSequence text = mBuilder.processLegacyText(extras.getCharSequence(EXTRA_TEXT)); if (text == null) { text = getDefaultText(); @@ -9452,22 +9463,26 @@ public class Notification implements Parcelable .callStyleActions(true) .allowTextWithProgress(true) .hideLargeIcon(true) + .hideAppName(isCollapsed) + .titleViewId(R.id.conversation_text) + .title(title) .text(text) .summaryText(mBuilder.processLegacyText(mVerificationText)); mBuilder.mActions = getActionsListWithSystemActions(); final RemoteViews contentView; - if (p.mViewType != StandardTemplateParams.VIEW_TYPE_NORMAL) { - contentView = mBuilder.applyStandardTemplateWithActions( - R.layout.notification_template_material_big_call, p, null /* result */); - } else { + if (isCollapsed) { contentView = mBuilder.applyStandardTemplate( R.layout.notification_template_material_call, p, null /* result */); + } else { + contentView = mBuilder.applyStandardTemplateWithActions( + R.layout.notification_template_material_big_call, p, null /* result */); } // Bind some extra conversation-specific header fields. - mBuilder.setTextViewColorPrimary(contentView, R.id.conversation_text, p); - mBuilder.setTextViewColorSecondary(contentView, R.id.app_name_divider, p); - contentView.setViewVisibility(R.id.app_name_divider, View.VISIBLE); + if (!p.mHideAppName) { + mBuilder.setTextViewColorSecondary(contentView, R.id.app_name_divider, p); + contentView.setViewVisibility(R.id.app_name_divider, View.VISIBLE); + } bindCallerVerification(contentView, p); // Bind some custom CallLayout properties @@ -12144,12 +12159,13 @@ public class Notification implements Parcelable public static int VIEW_TYPE_NORMAL = 1; public static int VIEW_TYPE_BIG = 2; public static int VIEW_TYPE_HEADS_UP = 3; - public static int VIEW_TYPE_MINIMIZED = 4; - public static int VIEW_TYPE_PUBLIC = 5; - public static int VIEW_TYPE_GROUP_HEADER = 6; + public static int VIEW_TYPE_MINIMIZED = 4; // header only for minimized state + public static int VIEW_TYPE_PUBLIC = 5; // header only for automatic public version + public static int VIEW_TYPE_GROUP_HEADER = 6; // header only for top of group int mViewType = VIEW_TYPE_UNSPECIFIED; boolean mHeaderless; + boolean mHideAppName; boolean mHideTitle; boolean mHideActions; boolean mHideProgress; @@ -12157,6 +12173,7 @@ public class Notification implements Parcelable boolean mPromotePicture; boolean mCallStyleActions; boolean mAllowTextWithProgress; + int mTitleViewId; int mTextViewId; CharSequence title; CharSequence text; @@ -12170,6 +12187,7 @@ public class Notification implements Parcelable final StandardTemplateParams reset() { mViewType = VIEW_TYPE_UNSPECIFIED; mHeaderless = false; + mHideAppName = false; mHideTitle = false; mHideActions = false; mHideProgress = false; @@ -12177,6 +12195,7 @@ public class Notification implements Parcelable mPromotePicture = false; mCallStyleActions = false; mAllowTextWithProgress = false; + mTitleViewId = R.id.title; mTextViewId = R.id.text; title = null; text = null; @@ -12202,6 +12221,11 @@ public class Notification implements Parcelable return this; } + public StandardTemplateParams hideAppName(boolean hideAppName) { + mHideAppName = hideAppName; + return this; + } + final StandardTemplateParams hideActions(boolean hideActions) { this.mHideActions = hideActions; return this; @@ -12237,6 +12261,11 @@ public class Notification implements Parcelable return this; } + public StandardTemplateParams titleViewId(int titleViewId) { + mTitleViewId = titleViewId; + return this; + } + public StandardTemplateParams textViewId(int textViewId) { mTextViewId = textViewId; return this; diff --git a/core/java/com/android/internal/widget/CallLayout.java b/core/java/com/android/internal/widget/CallLayout.java index 6cc5a4aacda59..83345dad8ed9a 100644 --- a/core/java/com/android/internal/widget/CallLayout.java +++ b/core/java/com/android/internal/widget/CallLayout.java @@ -100,7 +100,6 @@ public class CallLayout extends FrameLayout { } // TODO(b/179178086): crop/clip the icon to a circle? mConversationIconView.setImageIcon(icon); - mConversationText.setText(callerName); } @RemotableViewMethod diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java index dabea582798fa..8c21e767c5c95 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java @@ -1346,9 +1346,7 @@ public class NotificationContentView extends FrameLayout { } ImageView bubbleButton = layout.findViewById(com.android.internal.R.id.bubble_button); View actionContainer = layout.findViewById(com.android.internal.R.id.actions_container); - LinearLayout actionContainerLayout = - layout.findViewById(com.android.internal.R.id.actions_container_layout); - if (bubbleButton == null || actionContainer == null || actionContainerLayout == null) { + if (bubbleButton == null || actionContainer == null) { return; } boolean isPersonWithShortcut = diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java index 2b194ba158168..dac3b8b739baf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java @@ -324,7 +324,7 @@ public class NotificationChildrenContainer extends ViewGroup { StatusBarNotification notification = mContainingNotification.getEntry().getSbn(); final Notification.Builder builder = Notification.Builder.recoverBuilder(getContext(), notification.getNotification()); - RemoteViews header = builder.makeNotificationHeader(); + RemoteViews header = builder.makeNotificationGroupHeader(); if (mNotificationHeader == null) { mNotificationHeader = (NotificationHeaderView) header.apply(getContext(), this); mNotificationHeader.findViewById(com.android.internal.R.id.expand_button)