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

Commit eec4afd0 authored by Mady Mellor's avatar Mady Mellor
Browse files

Fix an issue where if an app specified a height we wouldn't use it

The bubble API allows apps to specify a custom height for a bubble.
The recent changes to specify a fixed height for bubbles in large
screen landscape didn't account for custom heights.

This CL fixes the issue by returning the fixed height for large
screens in the getMaxExpandedViewHeight method -- that's what gets
called when the developer hasn't specified their own height.

This CL adds some tests for max height / app specified heights.

Additionally this change modifies the height calculation for large
screens slightly to:
- subtract the expanded view padding so that the view has a bit of
  space below the status bar & above the bottom inset
- account for the pointer similar to the other height calculations

Test: manual - go through CtsVerifier test for bubbles
Test: atest BubblePositionerTest
Bug: 310145069
Bug: 310805933
Change-Id: Ib1595894598ab605ee58fc6245d19f32ece58897
parent 2041a1e0
Loading
Loading
Loading
Loading
+13 −9
Original line number Diff line number Diff line
@@ -390,6 +390,9 @@ public class BubblePositioner {
     * the screen and the size of the elements around it (e.g. padding, pointer, manage button).
     */
    public int getMaxExpandedViewHeight(boolean isOverflow) {
        if (mDeviceConfig.isLargeScreen() && !mDeviceConfig.isSmallTablet() && !isOverflow) {
            return getExpandedViewHeightForLargeScreen();
        }
        // Subtract top insets because availableRect.height would account for that
        int expandedContainerY = (int) getExpandedViewYTopAligned() - getInsets().top;
        int paddingTop = showBubblesVertically()
@@ -407,6 +410,16 @@ public class BubblePositioner {
                - bottomPadding;
    }

    private int getExpandedViewHeightForLargeScreen() {
        // the expanded view height on large tablets is calculated based on the shortest screen
        // size and is the same in both portrait and landscape
        int maxVerticalInset = Math.max(mInsets.top, mInsets.bottom);
        int shortestScreenSide = Math.min(getScreenRect().height(), getScreenRect().width());
        // Subtract pointer size because it's laid out in LinearLayout with the expanded view.
        return shortestScreenSide - maxVerticalInset * 2
                - mManageButtonHeight - mPointerWidth - mExpandedViewPadding * 2;
    }

    /**
     * Determines the height for the bubble, ensuring a minimum height. If the height should be as
     * big as available, returns {@link #MAX_HEIGHT}.
@@ -417,15 +430,6 @@ public class BubblePositioner {
            // overflow in landscape on phone is max
            return MAX_HEIGHT;
        }

        if (mDeviceConfig.isLargeScreen() && !mDeviceConfig.isSmallTablet() && !isOverflow) {
            // the expanded view height on large tablets is calculated based on the shortest screen
            // size and is the same in both portrait and landscape
            int maxVerticalInset = Math.max(mInsets.top, mInsets.bottom);
            int shortestScreenSide = Math.min(mScreenRect.height(), mScreenRect.width());
            return shortestScreenSide - 2 * maxVerticalInset - mManageButtonHeight;
        }

        float desiredHeight = isOverflow
                ? mOverflowHeight
                : ((Bubble) bubble).getDesiredHeight(mContext);
+89 −3
Original line number Diff line number Diff line
@@ -16,10 +16,15 @@

package com.android.wm.shell.bubbles;

import static com.android.wm.shell.bubbles.BubblePositioner.MAX_HEIGHT;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;

import static org.mockito.Mockito.mock;

import android.content.Intent;
import android.content.pm.ShortcutInfo;
import android.graphics.Insets;
import android.graphics.PointF;
import android.graphics.Rect;
@@ -226,7 +231,7 @@ public class BubblePositionerTest extends ShellTestCase {
    }

    @Test
    public void testExpandedViewHeight_onLargeTablet() {
    public void testGetExpandedViewHeight_max() {
        Insets insets = Insets.of(10, 20, 5, 15);
        Rect screenBounds = new Rect(0, 0, 1800, 2600);

@@ -240,10 +245,91 @@ public class BubblePositionerTest extends ShellTestCase {
        Intent intent = new Intent(Intent.ACTION_VIEW).setPackage(mContext.getPackageName());
        Bubble bubble = Bubble.createAppBubble(intent, new UserHandle(1), null, directExecutor());

        assertThat(mPositioner.getExpandedViewHeight(bubble)).isEqualTo(MAX_HEIGHT);
    }

    @Test
    public void testGetExpandedViewHeight_customHeight_valid() {
        Insets insets = Insets.of(10, 20, 5, 15);
        Rect screenBounds = new Rect(0, 0, 1800, 2600);

        DeviceConfig deviceConfig = new ConfigBuilder()
                .setLargeScreen()
                .setInsets(insets)
                .setScreenBounds(screenBounds)
                .build();
        mPositioner.update(deviceConfig);

        final int minHeight = mContext.getResources().getDimensionPixelSize(
                R.dimen.bubble_expanded_default_height);
        Bubble bubble = new Bubble("key",
                mock(ShortcutInfo.class),
                minHeight + 100 /* desiredHeight */,
                0 /* desiredHeightResId */,
                "title",
                0 /* taskId */,
                null /* locus */,
                true /* isDismissable */,
                directExecutor(),
                mock(Bubbles.BubbleMetadataFlagListener.class));

        // Ensure the height is the same as the desired value
        assertThat(mPositioner.getExpandedViewHeight(bubble)).isEqualTo(
                bubble.getDesiredHeight(mContext));
    }


    @Test
    public void testGetExpandedViewHeight_customHeight_tooSmall() {
        Insets insets = Insets.of(10, 20, 5, 15);
        Rect screenBounds = new Rect(0, 0, 1800, 2600);

        DeviceConfig deviceConfig = new ConfigBuilder()
                .setLargeScreen()
                .setInsets(insets)
                .setScreenBounds(screenBounds)
                .build();
        mPositioner.update(deviceConfig);

        Bubble bubble = new Bubble("key",
                mock(ShortcutInfo.class),
                10 /* desiredHeight */,
                0 /* desiredHeightResId */,
                "title",
                0 /* taskId */,
                null /* locus */,
                true /* isDismissable */,
                directExecutor(),
                mock(Bubbles.BubbleMetadataFlagListener.class));

        // Ensure the height is the same as the minimum value
        final int minHeight = mContext.getResources().getDimensionPixelSize(
                R.dimen.bubble_expanded_default_height);
        assertThat(mPositioner.getExpandedViewHeight(bubble)).isEqualTo(minHeight);
    }

    @Test
    public void testGetMaxExpandedViewHeight_onLargeTablet() {
        Insets insets = Insets.of(10, 20, 5, 15);
        Rect screenBounds = new Rect(0, 0, 1800, 2600);

        DeviceConfig deviceConfig = new ConfigBuilder()
                .setLargeScreen()
                .setInsets(insets)
                .setScreenBounds(screenBounds)
                .build();
        mPositioner.update(deviceConfig);

        int manageButtonHeight =
                mContext.getResources().getDimensionPixelSize(R.dimen.bubble_manage_button_height);
        float expectedHeight = 1800 - 2 * 20 - manageButtonHeight;
        assertThat(mPositioner.getExpandedViewHeight(bubble)).isWithin(0.1f).of(expectedHeight);
        int pointerWidth = mContext.getResources().getDimensionPixelSize(
                R.dimen.bubble_pointer_width);
        int expandedViewPadding = mContext.getResources().getDimensionPixelSize(R
                .dimen.bubble_expanded_view_padding);
        float expectedHeight = 1800 - 2 * 20 - manageButtonHeight - pointerWidth
                - expandedViewPadding * 2;
        assertThat(((float) mPositioner.getMaxExpandedViewHeight(false /* isOverflow */)))
                .isWithin(0.1f).of(expectedHeight);
    }

    /**