From b642af90a18f7da1a7413aebe819bdb67576025f Mon Sep 17 00:00:00 2001 From: Johannes Gallmann Date: Fri, 14 Oct 2022 09:58:09 +0000 Subject: [PATCH] Fix notification inline reply animation This CL fixes the animation for the appearance of the inline reply RemoteInputView, bringing it as close to the Direct spec (https://direct.googleplex.com/#/spec/283080003) as possible. Intentional deviations from the Direct specs are the following: 1. The animation duration is 360 ms instead of 500 ms (in order to be consistent with other notification shade resize animations) 2. The interpolator is FAST_IN_SLOW_OUT, which might differ slightly from the Direct specs "DECELERATE_WITH_EASING" interpolator. (same reason as for 1.) 3. The height of the notification is increased slightly more than in the Direct spec. (This is done in order to keep the paddings and margins as they currently are. It seems to me that if there was more text in the Direct visualization, the top margin of the inline reply EditText would be too small.) This CL does not fix the following related issues: 1. The collapsed heads up view does not allocate enough space to fit the RemoteInputView. This causes the top part of the notification to get shrunk (Icons jump up by a few pixels), when clicking the "Reply" button. It also affects the beginning of the RemoteInputView expand animation to be partly shifted out of the HeadsUp View. 2. When defocusing the RemoteInputView, the "Reply" button is clipped during the beginning of the animation. 3. When focusing the RemoteInputView, the notification shade is not scrolled correctly. (Keyboard is overlapping RemoteInputView) 4. The expand/collapse animation is broken while the RemoteInputView is visible. I will address these issues in seperate CLs. Bug: 174148361 Test: atest RemoteInputViewTest, Manual, i.e. posting various types of Notifications from the Notify2-RVC application with a reply action added to them. Then observing the animations visually and analyzing frames from screen recordings. Change-Id: I8b2e821b254d6d7fdfbca22426eceafff95d5322 --- .../notification_material_action_list.xml | 1 + packages/SystemUI/Android.bp | 1 + packages/SystemUI/res/layout/remote_input.xml | 1 + .../src/com/android/systemui/flags/Flags.kt | 5 + .../NotificationRemoteInputManager.java | 6 +- .../row/NotificationContentView.java | 2 +- .../statusbar/policy/RemoteInputView.java | 194 ++++++++++++++++-- .../policy/RemoteInputViewController.kt | 11 +- .../statusbar/policy/RemoteInputViewTest.java | 68 +++++- 9 files changed, 268 insertions(+), 21 deletions(-) diff --git a/core/res/res/layout/notification_material_action_list.xml b/core/res/res/layout/notification_material_action_list.xml index a5608af7a0819..7aef82a8aa3a6 100644 --- a/core/res/res/layout/notification_material_action_list.xml +++ b/core/res/res/layout/notification_material_action_list.xml @@ -27,6 +27,7 @@ android:layout_width="match_parent" android:layout_height="wrap_content" android:gravity="end" + android:layout_gravity="bottom" android:orientation="horizontal" android:background="@color/notification_action_list_background_color" > diff --git a/packages/SystemUI/Android.bp b/packages/SystemUI/Android.bp index e624441ca7771..25fb24a3417fc 100644 --- a/packages/SystemUI/Android.bp +++ b/packages/SystemUI/Android.bp @@ -221,6 +221,7 @@ android_library { "WindowManager-Shell", "LowLightDreamLib", "motion_tool_lib", + "androidx.core_core-animation-testing-nodeps", ], } diff --git a/packages/SystemUI/res/layout/remote_input.xml b/packages/SystemUI/res/layout/remote_input.xml index a5b2f80a31587..f4b0a45a8d326 100644 --- a/packages/SystemUI/res/layout/remote_input.xml +++ b/packages/SystemUI/res/layout/remote_input.xml @@ -20,6 +20,7 @@ setAttachment(null)); - LinearLayout contentView = findViewById(R.id.remote_input_content); - contentView.setBackground(mContentBackground); + mContentView = findViewById(R.id.remote_input_content); + mContentView.setBackground(mContentBackground); mEditText = findViewById(R.id.remote_input_text); mEditText.setInnerFocusable(false); // TextView initializes the spell checked when the view is attached to a window. @@ -398,20 +418,70 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene return true; } - private void onDefocus(boolean animate, boolean logClose) { + /** + * View will ensure to use at most the provided defocusTargetHeight, when defocusing animated. + * This is to ensure that the parent can resize itself to the targetHeight while the defocus + * animation of the RemoteInputView is running. + * + * @param defocusTargetHeight The target height the parent will resize itself to. If null, the + * RemoteInputView will not resize itself. + */ + public void setDefocusTargetHeight(Integer defocusTargetHeight) { + mDefocusTargetHeight = defocusTargetHeight; + } + + @VisibleForTesting + void onDefocus(boolean animate, boolean logClose) { mController.removeRemoteInput(mEntry, mToken); mEntry.remoteInputText = mEditText.getText(); // During removal, we get reattached and lose focus. Not hiding in that // case to prevent flicker. if (!mRemoved) { - if (animate && mRevealParams != null && mRevealParams.radius > 0) { - Animator reveal = mRevealParams.createCircularHideAnimator(this); - reveal.setInterpolator(Interpolators.FAST_OUT_LINEAR_IN); - reveal.setDuration(StackStateAnimator.ANIMATION_DURATION_CLOSE_REMOTE_INPUT); - reveal.addListener(new AnimatorListenerAdapter() { + if (animate && mIsFocusAnimationFlagActive) { + Animator animator = getDefocusAnimator(); + + // When defocusing, the notification needs to shrink. Therefore, we need to free + // up the space that is needed for the RemoteInputView. This is done by setting + // a negative top margin of the height difference of the RemoteInputView and its + // sibling (the actions_container_layout containing the Reply button) + if (mDefocusTargetHeight != null && mDefocusTargetHeight < getHeight() + && mDefocusTargetHeight >= 0 + && getLayoutParams() instanceof FrameLayout.LayoutParams) { + int heightToShrink = getHeight() - mDefocusTargetHeight; + FrameLayout.LayoutParams layoutParams = + (FrameLayout.LayoutParams) getLayoutParams(); + layoutParams.topMargin = -heightToShrink; + setLayoutParams(layoutParams); + ((ViewGroup) getParent().getParent()).setClipChildren(false); + } + + animator.addListener(new AnimatorListenerAdapter() { @Override public void onAnimationEnd(Animator animation) { + //reset top margin after the animation + if (getLayoutParams() instanceof FrameLayout.LayoutParams) { + FrameLayout.LayoutParams layoutParams = + (FrameLayout.LayoutParams) getLayoutParams(); + layoutParams.topMargin = 0; + setLayoutParams(layoutParams); + ((ViewGroup) getParent().getParent()).setClipChildren(true); + } + setVisibility(GONE); + if (mWrapper != null) { + mWrapper.setRemoteInputVisible(false); + } + } + }); + animator.start(); + + } else if (animate && mRevealParams != null && mRevealParams.radius > 0) { + android.animation.Animator reveal = mRevealParams.createCircularHideAnimator(this); + reveal.setInterpolator(Interpolators.FAST_OUT_LINEAR_IN); + reveal.setDuration(StackStateAnimator.ANIMATION_DURATION_CLOSE_REMOTE_INPUT); + reveal.addListener(new android.animation.AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(android.animation.Animator animation) { setVisibility(GONE); if (mWrapper != null) { mWrapper.setRemoteInputVisible(false); @@ -533,12 +603,29 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene mEditText.setText(editTextContent); } - public void focusAnimated() { - if (getVisibility() != VISIBLE && mRevealParams != null) { - Animator animator = mRevealParams.createCircularRevealAnimator(this); + /** + * Sets whether the feature flag for the updated inline reply animation is active or not. + * @param active + */ + public void setIsFocusAnimationFlagActive(boolean active) { + mIsFocusAnimationFlagActive = active; + } + + /** + * Focuses the RemoteInputView and animates its appearance + * + * @param crossFadeView view that will be crossfaded during the appearance animation + */ + public void focusAnimated(View crossFadeView) { + if (!mIsFocusAnimationFlagActive && getVisibility() != VISIBLE + && mRevealParams != null) { + android.animation.Animator animator = mRevealParams.createCircularRevealAnimator(this); animator.setDuration(StackStateAnimator.ANIMATION_DURATION_STANDARD); animator.setInterpolator(Interpolators.LINEAR_OUT_SLOW_IN); animator.start(); + } else if (mIsFocusAnimationFlagActive && getVisibility() != VISIBLE) { + setAlpha(0f); + getFocusAnimator(crossFadeView).start(); } focus(); } @@ -737,6 +824,81 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene mOnSendListeners.remove(listener); } + @Override + protected void onLayout(boolean changed, int l, int t, int r, int b) { + super.onLayout(changed, l, t, r, b); + if (mIsFocusAnimationFlagActive) setPivotY(getMeasuredHeight()); + if (mContentBackgroundBounds != null) { + mContentBackground.setBounds(mContentBackgroundBounds); + } + } + + private Animator getFocusAnimator(View crossFadeView) { + final Animator alphaAnimator = ObjectAnimator.ofFloat(this, View.ALPHA, 0f, 1f); + alphaAnimator.setStartDelay(FOCUS_ANIMATION_FADE_IN_DELAY); + alphaAnimator.setDuration(FOCUS_ANIMATION_FADE_IN_DURATION); + alphaAnimator.setInterpolator(InterpolatorsAndroidX.LINEAR); + + ValueAnimator scaleAnimator = ValueAnimator.ofFloat(FOCUS_ANIMATION_MIN_SCALE, 1f); + scaleAnimator.addUpdateListener(valueAnimator -> { + setFocusAnimationScaleY((float) scaleAnimator.getAnimatedValue()); + }); + scaleAnimator.setDuration(FOCUS_ANIMATION_TOTAL_DURATION); + scaleAnimator.setInterpolator(InterpolatorsAndroidX.FAST_OUT_SLOW_IN); + + final Animator crossFadeViewAlphaAnimator = + ObjectAnimator.ofFloat(crossFadeView, View.ALPHA, 1f, 0f); + crossFadeViewAlphaAnimator.setDuration(FOCUS_ANIMATION_CROSSFADE_DURATION); + crossFadeViewAlphaAnimator.setInterpolator(InterpolatorsAndroidX.LINEAR); + alphaAnimator.addListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation, boolean isReverse) { + crossFadeView.setAlpha(1f); + } + }); + + final AnimatorSet animatorSet = new AnimatorSet(); + animatorSet.playTogether(alphaAnimator, scaleAnimator, crossFadeViewAlphaAnimator); + return animatorSet; + } + + private Animator getDefocusAnimator() { + final Animator alphaAnimator = ObjectAnimator.ofFloat(this, View.ALPHA, 1f, 0f); + alphaAnimator.setDuration(FOCUS_ANIMATION_CROSSFADE_DURATION); + alphaAnimator.setInterpolator(InterpolatorsAndroidX.LINEAR); + + ValueAnimator scaleAnimator = ValueAnimator.ofFloat(1f, FOCUS_ANIMATION_MIN_SCALE); + scaleAnimator.addUpdateListener(valueAnimator -> { + setFocusAnimationScaleY((float) scaleAnimator.getAnimatedValue()); + }); + scaleAnimator.setDuration(FOCUS_ANIMATION_TOTAL_DURATION); + scaleAnimator.setInterpolator(InterpolatorsAndroidX.FAST_OUT_SLOW_IN); + scaleAnimator.addListener(new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation, boolean isReverse) { + setFocusAnimationScaleY(1f); + } + }); + + final AnimatorSet animatorSet = new AnimatorSet(); + animatorSet.playTogether(alphaAnimator, scaleAnimator); + return animatorSet; + } + + /** + * Sets affected view properties for a vertical scale animation + * + * @param scaleY desired vertical view scale + */ + private void setFocusAnimationScaleY(float scaleY) { + int verticalBoundOffset = (int) ((1f - scaleY) * 0.5f * mContentView.getHeight()); + mContentBackgroundBounds = new Rect(0, verticalBoundOffset, mContentView.getWidth(), + mContentView.getHeight() - verticalBoundOffset); + mContentBackground.setBounds(mContentBackgroundBounds); + mContentView.setBackground(mContentBackground); + setTranslationY(verticalBoundOffset); + } + /** Handler for button click on send action in IME. */ private class EditorActionHandler implements TextView.OnEditorActionListener { @@ -991,11 +1153,11 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene this.radius = radius; } - Animator createCircularHideAnimator(View view) { + android.animation.Animator createCircularHideAnimator(View view) { return ViewAnimationUtils.createCircularReveal(view, centerX, centerY, radius, 0); } - Animator createCircularRevealAnimator(View view) { + android.animation.Animator createCircularRevealAnimator(View view) { return ViewAnimationUtils.createCircularReveal(view, centerX, centerY, 0, radius); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputViewController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputViewController.kt index f8451017b367b..22b4c9d81d256 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputViewController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputViewController.kt @@ -30,6 +30,8 @@ import android.util.Log import android.view.View import com.android.internal.logging.UiEventLogger import com.android.systemui.R +import com.android.systemui.flags.FeatureFlags +import com.android.systemui.flags.Flags.NOTIFICATION_INLINE_REPLY_ANIMATION import com.android.systemui.statusbar.NotificationRemoteInputManager import com.android.systemui.statusbar.RemoteInputController import com.android.systemui.statusbar.notification.collection.NotificationEntry @@ -61,6 +63,8 @@ interface RemoteInputViewController { var revealParams: RevealParams? + val isFocusAnimationFlagActive: Boolean + /** * Sets the smart reply that should be inserted in the remote input, or `null` if the user is * not editing a smart reply. @@ -117,7 +121,8 @@ class RemoteInputViewControllerImpl @Inject constructor( private val remoteInputQuickSettingsDisabler: RemoteInputQuickSettingsDisabler, private val remoteInputController: RemoteInputController, private val shortcutManager: ShortcutManager, - private val uiEventLogger: UiEventLogger + private val uiEventLogger: UiEventLogger, + private val mFlags: FeatureFlags ) : RemoteInputViewController { private val onSendListeners = ArraySet() @@ -149,6 +154,9 @@ class RemoteInputViewControllerImpl @Inject constructor( override val isActive: Boolean get() = view.isActive + override val isFocusAnimationFlagActive: Boolean + get() = mFlags.isEnabled(NOTIFICATION_INLINE_REPLY_ANIMATION) + override fun bind() { if (isBound) return isBound = true @@ -159,6 +167,7 @@ class RemoteInputViewControllerImpl @Inject constructor( view.setSupportedMimeTypes(it.allowedDataTypes) } view.setRevealParameters(revealParams) + view.setIsFocusAnimationFlagActive(isFocusAnimationFlagActive) view.addOnEditTextFocusChangedListener(onFocusChangeListener) view.addOnSendRemoteInputListener(onSendRemoteInputListener) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/RemoteInputViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/RemoteInputViewTest.java index 915e999c2646f..1fe2008d2abea 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/RemoteInputViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/RemoteInputViewTest.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.policy; import static android.view.ContentInfo.SOURCE_CLIPBOARD; +import static com.android.systemui.statusbar.notification.stack.StackStateAnimator.ANIMATION_DURATION_STANDARD; + import static com.google.common.truth.Truth.assertThat; import static junit.framework.Assert.assertEquals; @@ -57,6 +59,7 @@ import android.window.OnBackInvokedDispatcher; import android.window.WindowOnBackInvokedDispatcher; import androidx.annotation.NonNull; +import androidx.core.animation.AnimatorTestRule; import androidx.test.filters.SmallTest; import com.android.internal.logging.UiEventLogger; @@ -64,15 +67,19 @@ import com.android.internal.logging.testing.UiEventLoggerFake; import com.android.systemui.Dependency; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; +import com.android.systemui.flags.FakeFeatureFlags; +import com.android.systemui.flags.Flags; import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.RemoteInputController; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.NotificationTestHelper; +import com.android.systemui.statusbar.notification.stack.StackStateAnimator; import com.android.systemui.statusbar.phone.LightBarController; import org.junit.After; import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -99,6 +106,9 @@ public class RemoteInputViewTest extends SysuiTestCase { private BlockingQueueIntentReceiver mReceiver; private final UiEventLoggerFake mUiEventLoggerFake = new UiEventLoggerFake(); + @ClassRule + public static AnimatorTestRule mAnimatorTestRule = new AnimatorTestRule(); + @Before public void setUp() throws Exception { allowTestableLooperAsMainThread(); @@ -294,6 +304,9 @@ public class RemoteInputViewTest extends SysuiTestCase { /* invoke the captured callback */ onBackInvokedCallbackCaptor.getValue().onBackInvoked(); + /* wait for RemoteInputView disappear animation to finish */ + mAnimatorTestRule.advanceTimeBy(StackStateAnimator.ANIMATION_DURATION_STANDARD); + /* verify that the RemoteInputView goes away */ assertEquals(view.getVisibility(), View.GONE); } @@ -363,19 +376,70 @@ public class RemoteInputViewTest extends SysuiTestCase { mUiEventLoggerFake.eventId(1)); } + @Test + public void testFocusAnimation() throws Exception { + NotificationTestHelper helper = new NotificationTestHelper( + mContext, + mDependency, + TestableLooper.get(this)); + ExpandableNotificationRow row = helper.createRow(); + RemoteInputView view = RemoteInputView.inflate(mContext, null, row.getEntry(), mController); + bindController(view, row.getEntry()); + view.setVisibility(View.GONE); + + View crossFadeView = new View(mContext); + + // Start focus animation + view.focusAnimated(crossFadeView); + + // fast forward to end of animation + mAnimatorTestRule.advanceTimeBy(ANIMATION_DURATION_STANDARD); + + // assert that crossFadeView's alpha is reset to 1f after the animation (hidden behind + // RemoteInputView) + assertEquals(1f, crossFadeView.getAlpha()); + assertEquals(View.VISIBLE, view.getVisibility()); + assertEquals(1f, view.getAlpha()); + } + + @Test + public void testDefocusAnimation() throws Exception { + NotificationTestHelper helper = new NotificationTestHelper( + mContext, + mDependency, + TestableLooper.get(this)); + ExpandableNotificationRow row = helper.createRow(); + RemoteInputView view = RemoteInputView.inflate(mContext, null, row.getEntry(), mController); + bindController(view, row.getEntry()); + + // Start defocus animation + view.onDefocus(true, false); + assertEquals(View.VISIBLE, view.getVisibility()); + + // fast forward to end of animation + mAnimatorTestRule.advanceTimeBy(ANIMATION_DURATION_STANDARD); + + // assert that RemoteInputView is no longer visible + assertEquals(View.GONE, view.getVisibility()); + } + // NOTE: because we're refactoring the RemoteInputView and moving logic into the - // RemoteInputViewController, it's easiest to just test the system of the two classes together. + // RemoteInputViewController, it's easiest to just test the system of the two classes together. @NonNull private RemoteInputViewController bindController( RemoteInputView view, NotificationEntry entry) { + FakeFeatureFlags fakeFeatureFlags = new FakeFeatureFlags(); + fakeFeatureFlags.set(Flags.NOTIFICATION_INLINE_REPLY_ANIMATION, true); RemoteInputViewControllerImpl viewController = new RemoteInputViewControllerImpl( view, entry, mRemoteInputQuickSettingsDisabler, mController, mShortcutManager, - mUiEventLoggerFake); + mUiEventLoggerFake, + fakeFeatureFlags + ); viewController.bind(); return viewController; }