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

Commit a5906cf3 authored by Olivier St-Onge's avatar Olivier St-Onge
Browse files

Small fixes for Edit mode 

Includes:
- Removing the "Remove" pill above the current tiles grid
- Showing available tiles when dragging within the current tiles grid
- Adjust the dragging zones on large tiles to avoid visual jank
- Adjust the color for the back arrow in the top left corner

Fixes: 404584621
Fixes: 404884846
Fixes: 404877414
Flag: com.android.systemui.qs_ui_refactor_compose_fragment
Test: manually
Test: DragAndDropTest
Test: EditModeTest

Change-Id: I53ec9c3bfd8ab81a03ce391e2026e25f8732c577
parent 38818825

packages/SystemUI/src/com/android/systemui/qs/panels/ui/compose/DragAndDropState.kt

+1 −1
Original line number Diff line number Diff line
@@ -172,7 +172,7 @@ private fun DragAndDropEvent.toOffset(): Offset { 
private fun insertAfter(item: LazyGridItemInfo, offset: Offset): Boolean {

    // We want to insert the tile after the target if we're aiming at the end of a large tile

    // TODO(ostonge): Verify this behavior in RTL

    val itemCenter = item.offset.x + item.size.width * .75

    val itemCenter = item.offset.x + item.size.width * .5

    return item.span != 1 && offset.x > itemCenter

}

packages/SystemUI/src/com/android/systemui/qs/panels/ui/compose/EditTileListState.kt

+0 −6
Original line number Diff line number Diff line
@@ -80,12 +80,6 @@ class EditTileListState( 
        return _tiles.indexOfFirst { it is TileGridCell && it.tile.tileSpec == tileSpec }

    }



    fun isRemovable(tileSpec: TileSpec): Boolean {

        return _tiles.find {

            it is TileGridCell && it.tile.tileSpec == tileSpec && it.tile.isRemovable

        } != null

    }



    /** Resize the tile corresponding to the [TileSpec] to [toIcon] */

    fun resizeTile(tileSpec: TileSpec, toIcon: Boolean) {

        val fromIndex = indexOf(tileSpec)

packages/SystemUI/src/com/android/systemui/qs/panels/ui/compose/infinitegrid/EditTile.kt

+9 −39
Original line number Diff line number Diff line
@@ -35,7 +35,6 @@ import androidx.compose.foundation.LocalOverscrollFactory 
import androidx.compose.foundation.ScrollState

import androidx.compose.foundation.background

import androidx.compose.foundation.border

import androidx.compose.foundation.clickable

import androidx.compose.foundation.clipScrollableContainer

import androidx.compose.foundation.gestures.Orientation

import androidx.compose.foundation.gestures.detectTapGestures
@@ -56,20 +55,17 @@ import androidx.compose.foundation.layout.padding 
import androidx.compose.foundation.layout.requiredHeightIn

import androidx.compose.foundation.layout.size

import androidx.compose.foundation.layout.wrapContentHeight

import androidx.compose.foundation.layout.wrapContentSize

import androidx.compose.foundation.lazy.grid.GridCells

import androidx.compose.foundation.lazy.grid.LazyGridScope

import androidx.compose.foundation.lazy.grid.LazyGridState

import androidx.compose.foundation.lazy.grid.itemsIndexed

import androidx.compose.foundation.lazy.grid.rememberLazyGridState

import androidx.compose.foundation.rememberScrollState

import androidx.compose.foundation.shape.CircleShape

import androidx.compose.foundation.shape.RoundedCornerShape

import androidx.compose.foundation.verticalScroll

import androidx.compose.material.icons.Icons

import androidx.compose.material.icons.automirrored.filled.ArrowBack

import androidx.compose.material.icons.filled.Add

import androidx.compose.material.icons.filled.Clear

import androidx.compose.material3.ButtonDefaults

import androidx.compose.material3.ExperimentalMaterial3Api

import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi
@@ -182,7 +178,7 @@ object TileType 
@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class)

@Composable

private fun EditModeTopBar(onStopEditing: () -> Unit, onReset: (() -> Unit)?) {

    val primaryContainerColor = MaterialTheme.colorScheme.primaryContainer

    val surfaceEffect2 = LocalAndroidColorScheme.current.surfaceEffect2

    TopAppBar(

        colors =

            TopAppBarDefaults.topAppBarColors(
@@ -199,7 +195,7 @@ private fun EditModeTopBar(onStopEditing: () -> Unit, onReset: (() -> Unit)?) { 
        navigationIcon = {

            IconButton(

                onClick = onStopEditing,

                modifier = Modifier.drawBehind { drawCircle(primaryContainerColor) },

                modifier = Modifier.drawBehind { drawCircle(surfaceEffect2) },

            ) {

                Icon(

                    Icons.AutoMirrored.Filled.ArrowBack,
@@ -305,7 +301,6 @@ fun DefaultEditTileGrid( 
                CurrentTilesGridHeader(

                    listState,

                    selectionState,

                    onRemoveTile,

                    modifier = Modifier.fillMaxWidth().heightIn(min = 48.dp),

                )
@@ -317,10 +312,16 @@ fun DefaultEditTileGrid( 
                        .requiredHeightIn(AvailableTilesGridMinHeight)

                        .animateContentSize()

                ) {

                    // Only show available tiles when a drag or placement isn't in progress, OR

                    // the drag is within the current tiles grid

                    val showAvailableTiles =

                        !(listState.dragInProgress || selectionState.placementEnabled) ||

                            listState.dragType == DragType.Move



                    // Using the fully qualified name here as a workaround for AnimatedVisibility

                    // not being available from a Box

                    androidx.compose.animation.AnimatedVisibility(

                        visible = !listState.dragInProgress && !selectionState.placementEnabled,

                        visible = showAvailableTiles,

                        enter = fadeIn(),

                        exit = fadeOut(),

                    ) {
@@ -404,7 +405,6 @@ private fun AutoScrollGrid( 
}



private enum class EditModeHeaderState {

    Remove,

    Place,

    Idle,

}
@@ -420,14 +420,9 @@ private fun rememberEditModeState( 
        selectionState.selected,

        selectionState.placementEnabled,

    ) {

        val canRemove =

            listState.isDraggedCellRemovable ||

                selectionState.selection?.let { listState.isRemovable(it) } ?: false



        editGridHeaderState.value =

            when {

                selectionState.placementEnabled -> EditModeHeaderState.Place

                canRemove -> EditModeHeaderState.Remove

                else -> EditModeHeaderState.Idle

            }

    }
@@ -439,7 +434,6 @@ private fun rememberEditModeState( 
private fun CurrentTilesGridHeader(

    listState: EditTileListState,

    selectionState: MutableSelectionState,

    onRemoveTile: (TileSpec) -> Unit,

    modifier: Modifier = Modifier,

) {

    val editGridHeaderState by rememberEditModeState(listState, selectionState)
@@ -452,14 +446,6 @@ private fun CurrentTilesGridHeader( 
    ) { state ->

        EditGridHeader {

            when (state) {

                EditModeHeaderState.Remove -> {

                    RemoveTileTarget {

                        selectionState.selection?.let {

                            selectionState.unSelect()

                            onRemoveTile(it)

                        }

                    }

                }

                EditModeHeaderState.Place -> {

                    EditGridCenteredText(text = stringResource(id = R.string.tap_to_position_tile))

                }
@@ -488,22 +474,6 @@ private fun EditGridCenteredText(text: String, modifier: Modifier = Modifier) { 
    Text(text = text, style = MaterialTheme.typography.titleSmall, modifier = modifier)

}



@Composable

private fun RemoveTileTarget(onClick: () -> Unit) {

    Row(

        verticalAlignment = Alignment.CenterVertically,

        horizontalArrangement = tileHorizontalArrangement(),

        modifier =

            Modifier.wrapContentSize()

                .clickable(onClick = onClick)

                .border(1.dp, LocalContentColor.current, shape = CircleShape)

                .padding(10.dp),

    ) {

        Icon(imageVector = Icons.Default.Clear, contentDescription = null)

        Text(text = stringResource(id = R.string.qs_customize_remove))

    }

}



@Composable

private fun CurrentTilesGrid(

    listState: EditTileListState,

packages/SystemUI/tests/src/com/android/systemui/qs/panels/ui/compose/DragAndDropTest.kt

+14 −57
Original line number Diff line number Diff line
@@ -19,10 +19,10 @@ package com.android.systemui.qs.panels.ui.compose 
import androidx.compose.foundation.layout.fillMaxSize

import androidx.compose.runtime.Composable

import androidx.compose.ui.Modifier

import androidx.compose.ui.test.assertCountEquals

import androidx.compose.ui.test.junit4.createComposeRule

import androidx.compose.ui.test.onNodeWithContentDescription

import androidx.compose.ui.test.onAllNodesWithContentDescription

import androidx.compose.ui.test.onNodeWithTag

import androidx.compose.ui.test.onNodeWithText

import androidx.compose.ui.text.AnnotatedString

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.FlakyTest
@@ -80,14 +80,12 @@ class DragAndDropTest : SysuiTestCase() { 


        listState.onStarted(listState.tiles[0] as TileGridCell, DragType.Move)



        // Tile is being dragged, it should be replaced with a placeholder

        composeRule.onNodeWithContentDescription("tileA").assertDoesNotExist()

        // Tile is being dragged, it should be replaced with a placeholder. Assert that only the

        // copy in the available section is visible

        composeRule.onAllNodesWithContentDescription("tileA").assertCountEquals(1)



        // Available tiles should disappear

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertDoesNotExist()



        // Remove drop zone should appear

        composeRule.onNodeWithText("Remove").assertExists()

        // Available tiles should still appear for a move

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()



        // Every other tile should still be in the same order

        composeRule.assertGridContainsExactly(
@@ -96,30 +94,6 @@ class DragAndDropTest : SysuiTestCase() { 
        )

    }



    @Test

    fun nonRemovableDraggedTile_removeHeaderShouldNotExist() {

        val nonRemovableTile = createEditTile("tileA", isRemovable = false)

        val listState =

            EditTileListState(

                listOf(nonRemovableTile),

                TestLargeTilesSpecs,

                columns = 4,

                largeTilesSpan = 2,

            )

        composeRule.setContent { EditTileGridUnderTest(listState) }

        composeRule.waitForIdle()



        val sizedTile = SizedTileImpl(nonRemovableTile, width = 1)

        listState.onStarted(sizedTile, DragType.Move)



        // Tile is being dragged, it should be replaced with a placeholder

        composeRule.onNodeWithContentDescription("tileA").assertDoesNotExist()



        // Remove drop zone should not appear

        composeRule.onNodeWithText("Remove").assertDoesNotExist()

    }



    @Test

    fun droppedNonRemovableDraggedTile_shouldStayInGrid() {

        val nonRemovableTile = createEditTile("tileA", isRemovable = false)

        val listState =
@@ -135,11 +109,9 @@ class DragAndDropTest : SysuiTestCase() { 
        val sizedTile = SizedTileImpl(nonRemovableTile, width = 1)

        listState.onStarted(sizedTile, DragType.Move)



        // Tile is being dragged, it should be replaced with a placeholder

        composeRule.onNodeWithContentDescription("tileA").assertDoesNotExist()



        // Remove drop zone should not appear

        composeRule.onNodeWithText("Remove").assertDoesNotExist()

        // Tile is being dragged, it should be replaced with a placeholder. Assert that only the

        // copy in the available section is visible

        composeRule.onAllNodesWithContentDescription("tileA").assertCountEquals(1)



        // Drop tile outside of the grid

        listState.movedOutOfBounds()
@@ -158,18 +130,12 @@ class DragAndDropTest : SysuiTestCase() { 


        listState.onStarted(listState.tiles[0] as TileGridCell, DragType.Move)



        // Remove drop zone should appear

        composeRule.onNodeWithText("Remove").assertExists()



        listState.onTargeting(1, false)

        listState.onDrop()



        // Available tiles should re-appear

        // Available tiles should appear for a move

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()



        // Remove drop zone should disappear

        composeRule.onNodeWithText("Remove").assertDoesNotExist()



        // Tile A and B should swap places

        composeRule.assertGridContainsExactly(

            CURRENT_TILES_GRID_TEST_TAG,
@@ -186,18 +152,12 @@ class DragAndDropTest : SysuiTestCase() { 


        listState.onStarted(listState.tiles[0] as TileGridCell, DragType.Move)



        // Remove drop zone should appear

        composeRule.onNodeWithText("Remove").assertExists()



        listState.movedOutOfBounds()

        listState.onDrop()



        // Available tiles should re-appear

        // Available tiles should appear for a move

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()



        // Remove drop zone should disappear

        composeRule.onNodeWithText("Remove").assertDoesNotExist()



        // Tile A is gone

        composeRule.assertGridContainsExactly(

            CURRENT_TILES_GRID_TEST_TAG,
@@ -215,8 +175,8 @@ class DragAndDropTest : SysuiTestCase() { 
        val sizedTile = SizedTileImpl(createEditTile("tile_new", isRemovable = false), width = 1)

        listState.onStarted(sizedTile, DragType.Add)



        // Remove drop zone should appear

        composeRule.onNodeWithText("Remove").assertExists()

        // Available tiles should disappear for an addition

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertDoesNotExist()



        // Insert after tileD, which is at index 4

        // [ a ] [ b ] [ c ] [ empty ]
@@ -227,9 +187,6 @@ class DragAndDropTest : SysuiTestCase() { 
        // Available tiles should re-appear

        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()



        // Remove drop zone should disappear

        composeRule.onNodeWithText("Remove").assertDoesNotExist()



        // tile_new is added after tileD

        composeRule.assertGridContainsExactly(

            CURRENT_TILES_GRID_TEST_TAG,

packages/SystemUI/tests/src/com/android/systemui/qs/panels/ui/compose/EditModeTest.kt

+0 −31
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ import androidx.compose.ui.test.junit4.createComposeRule 
import androidx.compose.ui.test.onAllNodesWithText

import androidx.compose.ui.test.onFirst

import androidx.compose.ui.test.onNodeWithContentDescription

import androidx.compose.ui.test.onNodeWithText

import androidx.compose.ui.test.performClick

import androidx.compose.ui.test.performTouchInput

import androidx.compose.ui.text.AnnotatedString
@@ -95,36 +94,6 @@ class EditModeTest : SysuiTestCase() { 
        composeRule.assertAvailableTilesGridContainsExactly(TestEditTiles.map { it.tileSpec.spec })

    }



    @Test

    fun clickRemoveTarget_shouldRemoveSelection() {

        composeRule.setContent { EditTileGridUnderTest(TestEditTiles) }

        composeRule.waitForIdle()



        // Selects first "tileA", i.e. the one in the current grid

        composeRule.onAllNodesWithText("tileA").onFirst().performClick()

        composeRule.onNodeWithText("Remove").performClick() // Removes



        composeRule.waitForIdle()



        composeRule.assertCurrentTilesGridContainsExactly(

            listOf("tileB", "tileC", "tileD_large", "tileE")

        )

        composeRule.assertAvailableTilesGridContainsExactly(TestEditTiles.map { it.tileSpec.spec })

    }



    @Test

    fun selectNonRemovableTile_removeTargetShouldHide() {

        val nonRemovableTile = createEditTile("tileA", isRemovable = false)

        composeRule.setContent { EditTileGridUnderTest(listOf(nonRemovableTile)) }

        composeRule.waitForIdle()



        // Selects first "tileA", i.e. the one in the current grid

        composeRule.onAllNodesWithText("tileA").onFirst().performClick()



        // Assert the remove target isn't shown

        composeRule.onNodeWithText("Remove").assertDoesNotExist()

    }



    @Test

    fun placementMode_shouldRepositionTile() {

        composeRule.setContent { EditTileGridUnderTest(TestEditTiles) }