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

Commit 83829be2 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[SB][Chips] Reduce padding in the normal icon + text chips. 

This is to make the call chip padding & screen record chip padding look
similar (b/397494654). Mostly, this re-writes a few things:
 - Renames the old padding dimen to _legacy
 - Updates the ChipIcon data class to say whether it has embedded
   padding
 - Adds some comments about how paddings work (when I originally wrote
   this CL, I tried to simplify things but ended up making it worse
   because I forgot why the paddings were in certain places)
 - Always apply the minimum width, but make sure the min width uses
   2*padding

Luckily the screenshot tests will show that most chips are unchanged.

Fixes: 397494654
Bug: 364653005
Flag: com.android.systemui.status_bar_chips_modernization
Test: Trigger screen record chip -> verify it has slightly less side
padding
Test: Verify other chips still have the same padding as before

Change-Id: Iab3d4f043f008fdef1a158e37391007afe6630ce
parent 546dbf91

packages/SystemUI/res/layout/ongoing_activity_chip_content.xml

+2 −2
Original line number Diff line number Diff line
@@ -25,8 +25,8 @@ 
        android:layout_gravity="center_vertical"

        android:gravity="center"

        android:background="@drawable/ongoing_activity_chip_bg"

        android:paddingStart="@dimen/ongoing_activity_chip_side_padding"

        android:paddingEnd="@dimen/ongoing_activity_chip_side_padding"

        android:paddingStart="@dimen/ongoing_activity_chip_side_padding_legacy"

        android:paddingEnd="@dimen/ongoing_activity_chip_side_padding_legacy"

        android:minWidth="@dimen/min_clickable_item_size"

    >

packages/SystemUI/res/values/dimens.xml

+2 −2
Original line number Diff line number Diff line
@@ -1838,8 +1838,8 @@ 
    <dimen name="ongoing_activity_chip_min_text_width">12dp</dimen>

    <dimen name="ongoing_activity_chip_max_text_width">74dp</dimen>

    <dimen name="ongoing_activity_chip_margin_start">5dp</dimen>

    <!-- The activity chip side padding, used with the default phone icon. -->

    <dimen name="ongoing_activity_chip_side_padding">12dp</dimen>

    <!-- The activity chip side padding, used in the legacy non-Compose chips with an icon that does not have embedded padding. -->

    <dimen name="ongoing_activity_chip_side_padding_legacy">12dp</dimen>

    <!-- The activity chip side padding, used with an icon that has embedded padding (e.g. if the icon comes from the notification's smallIcon field). If the icon has padding, the chip itself can have less padding. -->

    <dimen name="ongoing_activity_chip_side_padding_for_embedded_padding_icon">2dp</dimen>

    <dimen name="ongoing_activity_chip_side_padding_for_embedded_padding_icon_legacy">6dp</dimen>

packages/SystemUI/src/com/android/systemui/statusbar/chips/ui/binder/OngoingActivityChipBinder.kt

+3 −1
Original line number Diff line number Diff line
@@ -437,7 +437,9 @@ object OngoingActivityChipBinder { 


    private fun View.setBackgroundPaddingForNormalIcon() {

        val sidePadding =

            context.resources.getDimensionPixelSize(R.dimen.ongoing_activity_chip_side_padding)

            context.resources.getDimensionPixelSize(

                R.dimen.ongoing_activity_chip_side_padding_legacy

            )

        setPaddingRelative(sidePadding, paddingTop, sidePadding, paddingBottom)

    }

packages/SystemUI/src/com/android/systemui/statusbar/chips/ui/compose/ChipContent.kt

+8 −8
Original line number Diff line number Diff line
@@ -50,21 +50,21 @@ import kotlin.math.min 
fun ChipContent(viewModel: OngoingActivityChipModel.Active, modifier: Modifier = Modifier) {

    val context = LocalContext.current

    val density = LocalDensity.current

    val isTextOnly = viewModel.icon == null

    val hasEmbeddedIcon =

        viewModel.icon is OngoingActivityChipModel.ChipIcon.StatusBarView ||

            viewModel.icon is OngoingActivityChipModel.ChipIcon.StatusBarNotificationIcon

    val textStyle = MaterialTheme.typography.labelLargeEmphasized

    val textColor = Color(viewModel.colors.text(context))

    val maxTextWidth = dimensionResource(id = R.dimen.ongoing_activity_chip_max_text_width)

    val icon = viewModel.icon

    val startPadding =

        if (isTextOnly || hasEmbeddedIcon) {

            0.dp

        } else {

        if (icon != null && !icon.hasEmbeddedPadding) {

            // Add padding only if this text is next to an icon that doesn't embed its own padding

            dimensionResource(id = R.dimen.ongoing_activity_chip_icon_text_padding)

        } else {

            0.dp

        }

    // Include endPadding in the Text instead of the outer OngoingActivityChip so that if the text

    // is hidden because it's too large, then the remaining icon is still centered

    val endPadding =

        if (hasEmbeddedIcon) {

        if (icon?.hasEmbeddedPadding == true) {

            dimensionResource(

                id = R.dimen.ongoing_activity_chip_text_end_padding_for_embedded_padding_icon

            )

packages/SystemUI/src/com/android/systemui/statusbar/chips/ui/compose/OngoingActivityChip.kt

+13 −13
Original line number Diff line number Diff line
@@ -41,6 +41,8 @@ import androidx.compose.ui.res.dimensionResource 
import androidx.compose.ui.semantics.contentDescription

import androidx.compose.ui.semantics.semantics

import androidx.compose.ui.unit.Dp

import androidx.compose.ui.unit.dp

import androidx.compose.ui.unit.times

import androidx.compose.ui.viewinterop.AndroidView

import com.android.compose.animation.Expandable

import com.android.compose.modifiers.thenIf
@@ -87,14 +89,15 @@ fun OngoingActivityChip( 
        }

    val isClickable = onClick != null



    val chipSidePadding = dimensionResource(id = R.dimen.ongoing_activity_chip_side_padding)

    val chipSidePaddingTotal = 20.dp

    val minWidth =

        if (isClickable) {

            dimensionResource(id = R.dimen.min_clickable_item_size)

        } else if (model.icon != null) {

            dimensionResource(id = R.dimen.ongoing_activity_chip_icon_size) + chipSidePadding

            dimensionResource(id = R.dimen.ongoing_activity_chip_icon_size) + chipSidePaddingTotal

        } else {

            dimensionResource(id = R.dimen.ongoing_activity_chip_min_text_width) + chipSidePadding

            dimensionResource(id = R.dimen.ongoing_activity_chip_min_text_width) +

                chipSidePaddingTotal

        }



    Expandable(
@@ -109,7 +112,7 @@ fun OngoingActivityChip( 
                        this.contentDescription = contentDescription

                    }

                }

                .thenIf(isClickable) { Modifier.widthIn(min = minWidth) }

                .widthIn(min = minWidth)

                // For non-privacy-related chips, only show the chip if there's enough space for at

                // least the minimum width.

                .thenIf(!model.isImportantForPrivacy) {
@@ -138,7 +141,7 @@ fun OngoingActivityChip( 
        defaultMinSize = false,

        transitionControllerFactory = model.transitionManager?.controllerFactory,

    ) {

        ChipBody(model, iconViewStore, isClickable = isClickable, minWidth = minWidth)

        ChipBody(model, iconViewStore, minWidth = minWidth)

    }

}
@@ -146,14 +149,9 @@ fun OngoingActivityChip( 
private fun ChipBody(

    model: OngoingActivityChipModel.Active,

    iconViewStore: NotificationIconContainerViewBinder.IconViewStore?,

    isClickable: Boolean,

    minWidth: Dp,

    modifier: Modifier = Modifier,

) {

    val hasEmbeddedIcon =

        model.icon is OngoingActivityChipModel.ChipIcon.StatusBarView ||

            model.icon is OngoingActivityChipModel.ChipIcon.StatusBarNotificationIcon



    Row(

        horizontalArrangement = Arrangement.Center,

        verticalAlignment = Alignment.CenterVertically,
@@ -162,15 +160,17 @@ private fun ChipBody( 
                .fillMaxHeight()

                // Set the minWidth here as well as on the Expandable so that the content within

                // this row is still centered correctly horizontally

                .thenIf(isClickable) { Modifier.widthIn(min = minWidth) }

                .widthIn(min = minWidth)

                .padding(

                    // Always keep start & end padding the same so that if the text has to hide for

                    // some reason, the content is still centered

                    horizontal =

                        if (hasEmbeddedIcon) {

                        if (model.icon?.hasEmbeddedPadding == true) {

                            dimensionResource(

                                R.dimen.ongoing_activity_chip_side_padding_for_embedded_padding_icon

                            )

                        } else {

                            dimensionResource(id = R.dimen.ongoing_activity_chip_side_padding)

                            6.dp

                        }

                ),

    ) {