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

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

Simplify edit tile list state 

The list state now only tracks the current tiles because the available tiles are hidden during a drag movement. This simplifies the logic in EditTileListState and helps with unnecessary recompositions

Bug: 346991759
Flag: com.android.systemui.qs_ui_refactor
Test: EditTileListStateTest
Test: DragAndDropStateTest
Change-Id: Id51bc657038201e4edb89e10d9a3e70246574bb6
parent 7099f815

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

+8 −13
Original line number Diff line number Diff line
@@ -40,21 +40,19 @@ class DragAndDropStateTest : SysuiTestCase() { 
        TestEditTiles.forEach { assertThat(underTest.isMoving(it.tileSpec)).isFalse() }



        // Start the drag movement

        val movingTileSpec = TestEditTiles[0].tileSpec

        underTest.onStarted(movingTileSpec)

        underTest.onStarted(TestEditTiles[0])



        // Assert that the correct tile is marked as moving

        TestEditTiles.forEach {

            assertThat(underTest.isMoving(it.tileSpec)).isEqualTo(movingTileSpec == it.tileSpec)

            assertThat(underTest.isMoving(it.tileSpec))

                .isEqualTo(TestEditTiles[0].tileSpec == it.tileSpec)

        }

    }



    @Test

    fun onMoved_updatesList() {

        val movingTileSpec = TestEditTiles[0].tileSpec



        // Start the drag movement

        underTest.onStarted(movingTileSpec)

        underTest.onStarted(TestEditTiles[0])



        // Move the tile to the end of the list

        underTest.onMoved(listState.tiles[5].tileSpec)
@@ -67,10 +65,8 @@ class DragAndDropStateTest : SysuiTestCase() { 


    @Test

    fun onDrop_resetsMovingTile() {

        val movingTileSpec = TestEditTiles[0].tileSpec



        // Start the drag movement

        underTest.onStarted(movingTileSpec)

        underTest.onStarted(TestEditTiles[0])



        // Move the tile to the end of the list

        underTest.onMoved(listState.tiles[5].tileSpec)
@@ -84,16 +80,15 @@ class DragAndDropStateTest : SysuiTestCase() { 


    @Test

    fun onMoveOutOfBounds_removeMovingTileFromCurrentList() {

        val movingTileSpec = TestEditTiles[0].tileSpec



        // Start the drag movement

        underTest.onStarted(movingTileSpec)

        underTest.onStarted(TestEditTiles[0])



        // Move the tile outside of the list

        underTest.movedOutOfBounds()



        // Asserts the moving tile is not current

        assertThat(listState.tiles.first { it.tileSpec == movingTileSpec }.isCurrent).isFalse()

        assertThat(listState.tiles.firstOrNull { it.tileSpec == TestEditTiles[0].tileSpec })

            .isNull()

    }



    companion object {

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

+14 −21
Original line number Diff line number Diff line
@@ -33,22 +33,25 @@ class EditTileListStateTest : SysuiTestCase() { 
    val underTest = EditTileListState(TestEditTiles)



    @Test

    fun movingNonExistentTile_listUnchanged() {

        underTest.move(TileSpec.create("other_tile"), TestEditTiles[0].tileSpec)

    fun movingNonExistentTile_tileAdded() {

        val newTile = createEditTile("other_tile", false)

        underTest.move(newTile, TestEditTiles[0].tileSpec)



        assertThat(underTest.tiles).containsExactly(*TestEditTiles.toTypedArray())

        assertThat(underTest.tiles[0]).isEqualTo(newTile)

        assertThat(underTest.tiles.subList(1, underTest.tiles.size))

            .containsExactly(*TestEditTiles.toTypedArray())

    }



    @Test

    fun movingTileToNonExistentTarget_listUnchanged() {

        underTest.move(TestEditTiles[0].tileSpec, TileSpec.create("other_tile"))

        underTest.move(TestEditTiles[0], TileSpec.create("other_tile"))



        assertThat(underTest.tiles).containsExactly(*TestEditTiles.toTypedArray())

    }



    @Test

    fun movingTileToItself_listUnchanged() {

        underTest.move(TestEditTiles[0].tileSpec, TestEditTiles[0].tileSpec)

        underTest.move(TestEditTiles[0], TestEditTiles[0].tileSpec)



        assertThat(underTest.tiles).containsExactly(*TestEditTiles.toTypedArray())

    }
@@ -56,7 +59,7 @@ class EditTileListStateTest : SysuiTestCase() { 
    @Test

    fun movingTileToSameSection_listUpdates() {

        // Move tile at index 0 to index 1. Tile 0 should remain current.

        underTest.move(TestEditTiles[0].tileSpec, TestEditTiles[1].tileSpec)

        underTest.move(TestEditTiles[0], TestEditTiles[1].tileSpec)



        // Assert the tiles 0 and 1 have changed places.

        assertThat(underTest.tiles[0]).isEqualTo(TestEditTiles[1])
@@ -67,22 +70,12 @@ class EditTileListStateTest : SysuiTestCase() { 
            .containsExactly(*TestEditTiles.subList(2, 5).toTypedArray())

    }



    @Test

    fun movingTileToDifferentSection_listAndTileUpdates() {

        // Move tile at index 0 to index 3. Tile 0 should no longer be current.

        underTest.move(TestEditTiles[0].tileSpec, TestEditTiles[3].tileSpec)



        // Assert tile 0 is now at index 3 and is no longer current.

        assertThat(underTest.tiles[3]).isEqualTo(TestEditTiles[0].copy(isCurrent = false))

    fun removingTile_listUpdates() {

        // Remove tile at index 0

        underTest.remove(TestEditTiles[0].tileSpec)



        // Assert previous tiles have shifted places

        assertThat(underTest.tiles[0]).isEqualTo(TestEditTiles[1])

        assertThat(underTest.tiles[1]).isEqualTo(TestEditTiles[2])

        assertThat(underTest.tiles[2]).isEqualTo(TestEditTiles[3])



        // Assert the rest of the list is unchanged

        assertThat(underTest.tiles.subList(4, 5))

            .containsExactly(*TestEditTiles.subList(4, 5).toTypedArray())

        // Assert the tile was removed

        assertThat(underTest.tiles).containsExactly(*TestEditTiles.subList(1, 6).toTypedArray())

    }



    companion object {

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

+18 −20
Original line number Diff line number Diff line
@@ -14,12 +14,9 @@ 
 * limitations under the License.

 */



@file:OptIn(ExperimentalFoundationApi::class)



package com.android.systemui.qs.panels.ui.compose



import android.content.ClipData

import androidx.compose.foundation.ExperimentalFoundationApi

import androidx.compose.foundation.draganddrop.dragAndDropSource

import androidx.compose.foundation.draganddrop.dragAndDropTarget

import androidx.compose.foundation.gestures.detectTapGestures
@@ -32,11 +29,12 @@ import androidx.compose.ui.draganddrop.DragAndDropEvent 
import androidx.compose.ui.draganddrop.DragAndDropTarget

import androidx.compose.ui.draganddrop.DragAndDropTransferData

import androidx.compose.ui.draganddrop.mimeTypes

import com.android.systemui.qs.panels.ui.viewmodel.EditTileViewModel

import com.android.systemui.qs.pipeline.shared.TileSpec



@Composable

fun rememberDragAndDropState(listState: EditTileListState): DragAndDropState {

    val sourceSpec: MutableState<TileSpec?> = remember { mutableStateOf(null) }

    val sourceSpec: MutableState<EditTileViewModel?> = remember { mutableStateOf(null) }

    return remember(listState) { DragAndDropState(sourceSpec, listState) }

}
@@ -45,7 +43,7 @@ fun rememberDragAndDropState(listState: EditTileListState): DragAndDropState { 
 * drop events.

 */

class DragAndDropState(

    val sourceSpec: MutableState<TileSpec?>,

    val sourceSpec: MutableState<EditTileViewModel?>,

    private val listState: EditTileListState

) {

    val dragInProgress: Boolean
@@ -53,15 +51,15 @@ class DragAndDropState( 


    /** Returns index of the dragged tile if it's present in the list. Returns -1 if not. */

    fun currentPosition(): Int {

        return sourceSpec.value?.let { listState.indexOf(it) } ?: -1

        return sourceSpec.value?.let { listState.indexOf(it.tileSpec) } ?: -1

    }



    fun isMoving(tileSpec: TileSpec): Boolean {

        return sourceSpec.value?.let { it == tileSpec } ?: false

        return sourceSpec.value?.let { it.tileSpec == tileSpec } ?: false

    }



    fun onStarted(spec: TileSpec) {

        sourceSpec.value = spec

    fun onStarted(tile: EditTileViewModel) {

        sourceSpec.value = tile

    }



    fun onMoved(targetSpec: TileSpec) {
@@ -71,7 +69,7 @@ class DragAndDropState( 
    fun movedOutOfBounds() {

        // Removing the tiles from the current tile grid if it moves out of bounds. This clears

        // the spacer and makes it apparent that dropping the tile at that point would remove it.

        sourceSpec.value?.let { listState.removeFromCurrent(it) }

        sourceSpec.value?.let { listState.remove(it.tileSpec) }

    }



    fun onDrop() {
@@ -100,7 +98,7 @@ fun Modifier.dragAndDropTile( 
            object : DragAndDropTarget {

                override fun onDrop(event: DragAndDropEvent): Boolean {

                    return dragAndDropState.sourceSpec.value?.let {

                        onDrop(it, dragAndDropState.currentPosition())

                        onDrop(it.tileSpec, dragAndDropState.currentPosition())

                        dragAndDropState.onDrop()

                        true

                    } ?: false
@@ -114,7 +112,7 @@ fun Modifier.dragAndDropTile( 
    return dragAndDropTarget(

        shouldStartDragAndDrop = { event ->

            event.mimeTypes().contains(QsDragAndDrop.TILESPEC_MIME_TYPE) &&

                dragAndDropState.sourceSpec.value?.let { acceptDrops(it) } ?: false

                dragAndDropState.sourceSpec.value?.let { acceptDrops(it.tileSpec) } ?: false

        },

        target = target,

    )
@@ -137,7 +135,7 @@ fun Modifier.dragAndDropRemoveZone( 
            object : DragAndDropTarget {

                override fun onDrop(event: DragAndDropEvent): Boolean {

                    return dragAndDropState.sourceSpec.value?.let {

                        onDrop(it)

                        onDrop(it.tileSpec)

                        dragAndDropState.onDrop()

                        true

                    } ?: false
@@ -179,7 +177,7 @@ fun Modifier.dragAndDropTileList( 


                override fun onDrop(event: DragAndDropEvent): Boolean {

                    return dragAndDropState.sourceSpec.value?.let {

                        onDrop(it, dragAndDropState.currentPosition())

                        onDrop(it.tileSpec, dragAndDropState.currentPosition())

                        dragAndDropState.onDrop()

                        true

                    } ?: false
@@ -190,23 +188,23 @@ fun Modifier.dragAndDropTileList( 
        target = target,

        shouldStartDragAndDrop = { event ->

            event.mimeTypes().contains(QsDragAndDrop.TILESPEC_MIME_TYPE) &&

                dragAndDropState.sourceSpec.value?.let { acceptDrops(it) } ?: false

                dragAndDropState.sourceSpec.value?.let { acceptDrops(it.tileSpec) } ?: false

        },

    )

}



fun Modifier.dragAndDropTileSource(

    tileSpec: TileSpec,

    tile: EditTileViewModel,

    onTap: (TileSpec) -> Unit,

    onDoubleTap: (TileSpec) -> Unit,

    dragAndDropState: DragAndDropState

): Modifier {

    return dragAndDropSource {

        detectTapGestures(

            onTap = { onTap(tileSpec) },

            onDoubleTap = { onDoubleTap(tileSpec) },

            onTap = { onTap(tile.tileSpec) },

            onDoubleTap = { onDoubleTap(tile.tileSpec) },

            onLongPress = {

                dragAndDropState.onStarted(tileSpec)

                dragAndDropState.onStarted(tile)



                // The tilespec from the ClipData transferred isn't actually needed as we're moving

                // a tile within the same application. We're using a custom MIME type to limit the
@@ -216,7 +214,7 @@ fun Modifier.dragAndDropTileSource( 
                        ClipData(

                            QsDragAndDrop.CLIPDATA_LABEL,

                            arrayOf(QsDragAndDrop.TILESPEC_MIME_TYPE),

                            ClipData.Item(tileSpec.spec)

                            ClipData.Item(tile.tileSpec.spec)

                        )

                    )

                )

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

+12 −15
Original line number Diff line number Diff line
@@ -34,28 +34,25 @@ fun rememberEditListState( 
class EditTileListState(tiles: List<EditTileViewModel>) {

    val tiles: SnapshotStateList<EditTileViewModel> = tiles.toMutableStateList()



    fun move(tileSpec: TileSpec, target: TileSpec) {

        val fromIndex = indexOf(tileSpec)

    fun move(tile: EditTileViewModel, target: TileSpec) {

        val fromIndex = indexOf(tile.tileSpec)

        val toIndex = indexOf(target)



        if (fromIndex == -1 || toIndex == -1 || fromIndex == toIndex) {

        if (toIndex == -1 || fromIndex == toIndex) {

            return

        }



        val isMovingToCurrent = tiles[toIndex].isCurrent

        tiles.apply { add(toIndex, removeAt(fromIndex).copy(isCurrent = isMovingToCurrent)) }

        if (fromIndex == -1) {

            // If tile isn't in the list, simply insert it

            tiles.add(toIndex, tile)

        } else {

            // If tile is present in the list, move it

            tiles.apply { add(toIndex, removeAt(fromIndex)) }

        }



    /**

     * Sets the [TileSpec] as a non-current tile. Use this when a tile is dragged out of the current

     * tile grid.

     */

    fun removeFromCurrent(tileSpec: TileSpec) {

        val fromIndex = indexOf(tileSpec)

        if (fromIndex >= 0 && fromIndex < tiles.size) {

            // Mark the moving tile as non-current

            tiles[fromIndex] = tiles[fromIndex].copy(isCurrent = false)

    }



    fun remove(tileSpec: TileSpec) {

        tiles.removeIf { it.tileSpec == tileSpec }

    }



    fun indexOf(tileSpec: TileSpec): Int {

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

+6 −9
Original line number Diff line number Diff line
@@ -294,9 +294,9 @@ fun DefaultEditTileGrid( 
    onRemoveTile: (TileSpec) -> Unit,

    onResize: (TileSpec, Boolean) -> Unit,

) {

    val currentListState = rememberEditListState(tiles)

    val (currentTiles, otherTiles) = tiles.partition { it.isCurrent }

    val currentListState = rememberEditListState(currentTiles)

    val dragAndDropState = rememberDragAndDropState(currentListState)

    val (currentTiles, otherTiles) = currentListState.tiles.partition { it.isCurrent }



    val addTileToEnd: (TileSpec) -> Unit by rememberUpdatedState {

        onAddTile(it, CurrentTilesInteractor.POSITION_AT_END)
@@ -329,7 +329,7 @@ fun DefaultEditTileGrid( 
            }



            CurrentTilesGrid(

                currentTiles,

                currentListState.tiles,

                columns,

                tilePadding,

                isIconOnly,
@@ -480,16 +480,13 @@ private fun AvailableTileGrid( 
    onClick: (TileSpec) -> Unit,

    dragAndDropState: DragAndDropState,

) {

    val (otherTilesStock, otherTilesCustom) =

        tiles.filter { !dragAndDropState.isMoving(it.tileSpec) }.partition { it.appName == null }

    val (otherTilesStock, otherTilesCustom) = tiles.partition { it.appName == null }

    val availableTileHeight = tileHeight(true)

    val availableGridHeight = gridHeight(tiles.size, availableTileHeight, columns, tilePadding)



    // Available tiles

    TileLazyGrid(

        modifier =

            Modifier.height(availableGridHeight)

                .dragAndDropTileList(dragAndDropState, { false }, { _, _ -> }),

        modifier = Modifier.height(availableGridHeight),

        columns = GridCells.Fixed(columns)

    ) {

        editTiles(
@@ -594,7 +591,7 @@ fun LazyGridScope.editTiles( 
                        }

                        .dragAndDropTile(dragAndDropState, viewModel.tileSpec, acceptDrops, onDrop)

                        .dragAndDropTileSource(

                            viewModel.tileSpec,

                            viewModel,

                            onClick,

                            onDoubleTap,

                            dragAndDropState,