Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f2d325e5 authored by lyn's avatar lyn
Browse files

Fix scroll reset with bundle onboarding UI 

When a partially visible notification is removed from the top of
the shade (like during auto-grouping), the scroll position was
being incorrectly reset to an absolute value, causing a jarring
visual jump and potential for the scroll state to become corrupted.

This was most noticeable with Play Store download notifications
and the bundles onboarding UI, but the root cause is in
NSSL#updateScrollStateForRemovedChild

This change corrects the logic to perform a relative scroll
adjustment, preserving visual stability.

Fixes: 427074876
Test: atest SystemUiRoboTests:NotificationStackScrollLayoutTest
Test: see no scroll reset with onboarding UI
Flag: com.android.systemui.notification_bundle_ui

Change-Id: Ie9a020fe4b7212892a24e89a11bf3428f1e1c43c
parent 45dffd89

packages/SystemUI/multivalentTests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java

+65 −8
Original line number Diff line number Diff line
@@ -22,13 +22,10 @@ import static com.android.systemui.flags.SceneContainerFlagParameterizationKt.pa 
import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.ROWS_ALL;

import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.ROWS_GENTLE;

import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.RUBBER_BAND_FACTOR_NORMAL;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertTrue;



import static org.junit.Assert.assertFalse;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -104,8 +101,6 @@ import com.android.systemui.statusbar.phone.ScreenOffAnimationController; 
import com.android.systemui.statusbar.phone.StatusBarKeyguardViewManager;

import com.android.systemui.statusbar.policy.ResourcesSplitShadeStateController;



import kotlin.Unit;



import org.junit.Assert;

import org.junit.Before;

import org.junit.Rule;
@@ -116,13 +111,14 @@ import org.mockito.Mock; 
import org.mockito.junit.MockitoJUnit;

import org.mockito.junit.MockitoRule;



import platform.test.runner.parameterized.ParameterizedAndroidJunit4;

import platform.test.runner.parameterized.Parameters;



import java.util.ArrayList;

import java.util.List;

import java.util.function.Consumer;



import kotlin.Unit;

import platform.test.runner.parameterized.ParameterizedAndroidJunit4;

import platform.test.runner.parameterized.Parameters;



/**

 * Tests for {@link NotificationStackScrollLayout}.

 */
@@ -1990,6 +1986,67 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { 
        assertTrue("BottomOverlap not calculated accurately", firstRow.getBottomOverlap() == 0);

    }



    /**

     * This test validates the legacy behavior when the NotificationBundleUi flag is OFF.

     * It confirms that when a partially visible child is removed, the scroll position

     * incorrectly jumps to an absolute position based on the removed child's location.

     */

    @Test

    @DisableFlags(NotificationBundleUi.FLAG_NAME)

    @DisableSceneContainer

    public void updateScrollStateForRemovedChild_partiallyVisible_bundleUiFlagOff_jumpsToTop() {

        // GIVEN: A scrollable list with two rows, where we can control the height of the first row

        final int rowHeight = 200;

        ExpandableNotificationRow row1 = spy(mKosmos.createRow());

        doReturn(rowHeight).when(row1).getIntrinsicHeight();

        ExpandableNotificationRow row2 = mKosmos.createRow();

        mStackScroller.addContainerView(row1);

        mStackScroller.addContainerView(row2);



        // GIVEN: The NSSL is scrolled down by 100px, making row1 partially visible at the top.

        mStackScroller.setOwnScrollY(100);

        assertThat(mStackScroller.getOwnScrollY()).isEqualTo(100);



        // WHEN: The partially visible row1 is removed (simulating auto-grouping).

        mStackScroller.removeContainerView(row1);



        // THEN: The scroll position should jump to a value LESS THAN its original position.

        assertThat(mStackScroller.getOwnScrollY()).isLessThan(100);

    }



    /**

     * This test validates the new behavior when the NotificationBundleUi flag is ON.

     * It confirms that when a partially visible child is removed, the scroll position is

     * adjusted relatively to maintain visual stability, preventing any jump

     */

    @Test

    @EnableFlags(NotificationBundleUi.FLAG_NAME)

    @DisableSceneContainer

    public void updateScrollStateForRemovedChild_partiallyVisible_bundleUiFlagOn_adjustsRelative() {

        // GIVEN: A scrollable list with two rows, where we can control the height of the first.

        final int rowHeight = 200;

        final int padding = mContext.getResources()

                .getDimensionPixelSize(R.dimen.notification_divider_height);

        final int childHeight = rowHeight + padding;



        ExpandableNotificationRow row1 = spy(mKosmos.createRow());

        doReturn(rowHeight).when(row1).getIntrinsicHeight();

        ExpandableNotificationRow row2 = mKosmos.createRow();

        mStackScroller.addContainerView(row1);

        mStackScroller.addContainerView(row2);



        // GIVEN: The NSSL is scrolled down by 100px, making row1 partially visible at the top.

        mStackScroller.setOwnScrollY(100);

        assertThat(mStackScroller.getOwnScrollY()).isEqualTo(100);



        // WHEN: The partially visible row1 is removed (simulating auto-grouping).

        mStackScroller.removeContainerView(row1);



        // THEN: The scroll position should be adjusted by the height of the removed child to

        // maintain stability. The new scrollY will be (currentScrollY - childHeight).

        assertThat(mStackScroller.getOwnScrollY()).isEqualTo(100 - childHeight);

    }



    private MotionEvent captureTouchSentToSceneFramework() {

        ArgumentCaptor<MotionEvent> captor = ArgumentCaptor.forClass(MotionEvent.class);

        verify(mStackScrollLayoutController).sendTouchToSceneFramework(captor.capture());

packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java

+5 −1
Original line number Diff line number Diff line
@@ -3317,9 +3317,13 @@ public class NotificationStackScrollLayout 
        } else if (startingPosition < getOwnScrollY() - scrollBoundaryStart) {

            // This child is currently being scrolled into, set the scroll position to the

            // start of this child

            if (NotificationBundleUi.isEnabled()) {

                setOwnScrollY(getOwnScrollY() - childHeight);

            } else {

                setOwnScrollY(startingPosition + scrollBoundaryStart);

            }

        }

    }



    /**

     * @return the amount of scrolling needed to start clipping notifications.