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

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

Simplify the drag logic for edit mode.

The main changes here are:
- Use a single drag listener for the grid instead of one per tile. This simplifies the code and makes it easier to detect if the user is aiming at the left or right side of a tile
- Remove the tile list from the drag state and instead have the list listen to drag events
- Add spacers to empty spaces of the row to enable users to move a tile in that spot

Bug: 346991759
Flag: com.android.systemui.qs_ui_refactor
Test: DragAndDropStateTest
Test: EditTileListStateTest
Test: DragAndDropTest

Change-Id: I49cfdfe0f3c9a3513a4939867558be16d9ba9454
parent 484792a1
Loading
Loading
Loading
Loading
+0 −123
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

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

import androidx.compose.runtime.mutableStateOf
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.common.shared.model.Icon
import com.android.systemui.common.shared.model.Text
import com.android.systemui.qs.panels.shared.model.SizedTile
import com.android.systemui.qs.panels.shared.model.SizedTileImpl
import com.android.systemui.qs.panels.ui.viewmodel.EditTileViewModel
import com.android.systemui.qs.pipeline.shared.TileSpec
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith

@SmallTest
@RunWith(AndroidJUnit4::class)
class DragAndDropStateTest : SysuiTestCase() {
    private val listState = EditTileListState(TestEditTiles)
    private val underTest = DragAndDropState(mutableStateOf(null), listState)

    @Test
    fun isMoving_returnsCorrectValue() {
        // Asserts no tiles is moving
        TestEditTiles.forEach { assertThat(underTest.isMoving(it.tile.tileSpec)).isFalse() }

        // Start the drag movement
        underTest.onStarted(TestEditTiles[0])

        // Assert that the correct tile is marked as moving
        TestEditTiles.forEach {
            assertThat(underTest.isMoving(it.tile.tileSpec))
                .isEqualTo(TestEditTiles[0].tile.tileSpec == it.tile.tileSpec)
        }
    }

    @Test
    fun onMoved_updatesList() {
        // Start the drag movement
        underTest.onStarted(TestEditTiles[0])

        // Move the tile to the end of the list
        underTest.onMoved(listState.tiles[5].tile.tileSpec)
        assertThat(underTest.currentPosition()).isEqualTo(5)

        // Move the tile to the middle of the list
        underTest.onMoved(listState.tiles[2].tile.tileSpec)
        assertThat(underTest.currentPosition()).isEqualTo(2)
    }

    @Test
    fun onDrop_resetsMovingTile() {
        // Start the drag movement
        underTest.onStarted(TestEditTiles[0])

        // Move the tile to the end of the list
        underTest.onMoved(listState.tiles[5].tile.tileSpec)

        // Drop the tile
        underTest.onDrop()

        // Asserts no tiles is moving
        TestEditTiles.forEach { assertThat(underTest.isMoving(it.tile.tileSpec)).isFalse() }
    }

    @Test
    fun onMoveOutOfBounds_removeMovingTileFromCurrentList() {
        // Start the drag movement
        underTest.onStarted(TestEditTiles[0])

        // Move the tile outside of the list
        underTest.movedOutOfBounds()

        // Asserts the moving tile is not current
        assertThat(
                listState.tiles.firstOrNull { it.tile.tileSpec == TestEditTiles[0].tile.tileSpec }
            )
            .isNull()
    }

    companion object {
        private fun createEditTile(tileSpec: String): SizedTile<EditTileViewModel> {
            return SizedTileImpl(
                EditTileViewModel(
                    tileSpec = TileSpec.create(tileSpec),
                    icon = Icon.Resource(0, null),
                    label = Text.Loaded("unused"),
                    appName = null,
                    isCurrent = true,
                    availableEditActions = emptySet(),
                ),
                1,
            )
        }

        private val TestEditTiles =
            listOf(
                createEditTile("tileA"),
                createEditTile("tileB"),
                createEditTile("tileC"),
                createEditTile("tileD"),
                createEditTile("tileE"),
                createEditTile("tileF"),
            )
    }
}
+226 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

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

import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.test.assert
import androidx.compose.ui.test.hasContentDescription
import androidx.compose.ui.test.junit4.ComposeContentTestRule
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onChildAt
import androidx.compose.ui.test.onChildren
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.common.shared.model.ContentDescription
import com.android.systemui.common.shared.model.Icon
import com.android.systemui.common.shared.model.Text
import com.android.systemui.qs.panels.shared.model.SizedTile
import com.android.systemui.qs.panels.shared.model.SizedTileImpl
import com.android.systemui.qs.panels.ui.viewmodel.EditTileViewModel
import com.android.systemui.qs.pipeline.shared.TileSpec
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith

@SmallTest
@RunWith(AndroidJUnit4::class)
class DragAndDropTest : SysuiTestCase() {
    @get:Rule val composeRule = createComposeRule()

    // TODO(ostonge): Investigate why drag isn't detected when using performTouchInput
    @Composable
    private fun EditTileGridUnderTest(
        listState: EditTileListState,
        onSetTiles: (List<TileSpec>) -> Unit
    ) {
        DefaultEditTileGrid(
            currentListState = listState,
            otherTiles = listOf(),
            columns = 4,
            modifier = Modifier.fillMaxSize(),
            onAddTile = { _, _ -> },
            onRemoveTile = {},
            onSetTiles = onSetTiles,
            onResize = {},
        )
    }

    @Test
    fun draggedTile_shouldDisappear() {
        var tiles by mutableStateOf(TestEditTiles)
        val listState = EditTileListState(tiles, 4)
        composeRule.setContent {
            EditTileGridUnderTest(listState) {
                tiles = it.map { tileSpec -> createEditTile(tileSpec.spec) }
            }
        }
        composeRule.waitForIdle()

        listState.onStarted(TestEditTiles[0])

        // Tile is being dragged, it should be replaced with a placeholder
        composeRule.onNodeWithContentDescription("tileA").assertDoesNotExist()

        // Available tiles should disappear
        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertDoesNotExist()

        // Remove drop zone should appear
        composeRule.onNodeWithText("Remove").assertExists()

        // Every other tile should still be in the same order
        composeRule.assertTileGridContainsExactly(listOf("tileB", "tileC", "tileD_large", "tileE"))
    }

    @Test
    fun draggedTile_shouldChangePosition() {
        var tiles by mutableStateOf(TestEditTiles)
        val listState = EditTileListState(tiles, 4)
        composeRule.setContent {
            EditTileGridUnderTest(listState) {
                tiles = it.map { tileSpec -> createEditTile(tileSpec.spec) }
            }
        }
        composeRule.waitForIdle()

        listState.onStarted(TestEditTiles[0])
        listState.onMoved(1, false)
        listState.onDrop()

        // Available tiles should re-appear
        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.assertTileGridContainsExactly(
            listOf("tileB", "tileA", "tileC", "tileD_large", "tileE")
        )
    }

    @Test
    fun draggedTileOut_shouldBeRemoved() {
        var tiles by mutableStateOf(TestEditTiles)
        val listState = EditTileListState(tiles, 4)
        composeRule.setContent {
            EditTileGridUnderTest(listState) {
                tiles = it.map { tileSpec -> createEditTile(tileSpec.spec) }
            }
        }
        composeRule.waitForIdle()

        listState.onStarted(TestEditTiles[0])
        listState.movedOutOfBounds()
        listState.onDrop()

        // Available tiles should re-appear
        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()

        // Remove drop zone should disappear
        composeRule.onNodeWithText("Remove").assertDoesNotExist()

        // Tile A is gone
        composeRule.assertTileGridContainsExactly(listOf("tileB", "tileC", "tileD_large", "tileE"))
    }

    @Test
    fun draggedNewTileIn_shouldBeAdded() {
        var tiles by mutableStateOf(TestEditTiles)
        val listState = EditTileListState(tiles, 4)
        composeRule.setContent {
            EditTileGridUnderTest(listState) {
                tiles = it.map { tileSpec -> createEditTile(tileSpec.spec) }
            }
        }
        composeRule.waitForIdle()

        listState.onStarted(createEditTile("newTile"))
        // Insert after tileD, which is at index 4
        // [ a ] [ b ] [ c ] [ empty ]
        // [ tile d ] [ e ]
        listState.onMoved(4, insertAfter = true)
        listState.onDrop()

        // Available tiles should re-appear
        composeRule.onNodeWithTag(AVAILABLE_TILES_GRID_TEST_TAG).assertExists()

        // Remove drop zone should disappear
        composeRule.onNodeWithText("Remove").assertDoesNotExist()

        // newTile is added after tileD
        composeRule.assertTileGridContainsExactly(
            listOf("tileA", "tileB", "tileC", "tileD_large", "newTile", "tileE")
        )
    }

    private fun ComposeContentTestRule.assertTileGridContainsExactly(specs: List<String>) {
        onNodeWithTag(CURRENT_TILES_GRID_TEST_TAG).onChildren().apply {
            fetchSemanticsNodes().forEachIndexed { index, _ ->
                get(index).onChildAt(0).assert(hasContentDescription(specs[index]))
            }
        }
    }

    companion object {
        private const val CURRENT_TILES_GRID_TEST_TAG = "CurrentTilesGrid"
        private const val AVAILABLE_TILES_GRID_TEST_TAG = "AvailableTilesGrid"

        private fun createEditTile(tileSpec: String): SizedTile<EditTileViewModel> {
            return SizedTileImpl(
                EditTileViewModel(
                    tileSpec = TileSpec.create(tileSpec),
                    icon =
                        Icon.Resource(
                            android.R.drawable.star_on,
                            ContentDescription.Loaded(tileSpec)
                        ),
                    label = Text.Loaded(tileSpec),
                    appName = null,
                    isCurrent = true,
                    availableEditActions = emptySet(),
                ),
                getWidth(tileSpec),
            )
        }

        private fun getWidth(tileSpec: String): Int {
            return if (tileSpec.endsWith("large")) {
                2
            } else {
                1
            }
        }

        private val TestEditTiles =
            listOf(
                createEditTile("tileA"),
                createEditTile("tileB"),
                createEditTile("tileC"),
                createEditTile("tileD_large"),
                createEditTile("tileE"),
            )
    }
}
+93 −40
Original line number Diff line number Diff line
@@ -23,6 +23,9 @@ import com.android.systemui.common.shared.model.Icon
import com.android.systemui.common.shared.model.Text
import com.android.systemui.qs.panels.shared.model.SizedTile
import com.android.systemui.qs.panels.shared.model.SizedTileImpl
import com.android.systemui.qs.panels.ui.model.GridCell
import com.android.systemui.qs.panels.ui.model.SpacerGridCell
import com.android.systemui.qs.panels.ui.model.TileGridCell
import com.android.systemui.qs.panels.ui.viewmodel.EditTileViewModel
import com.android.systemui.qs.pipeline.shared.TileSpec
import com.google.common.truth.Truth.assertThat
@@ -32,80 +35,130 @@ import org.junit.runner.RunWith
@SmallTest
@RunWith(AndroidJUnit4::class)
class EditTileListStateTest : SysuiTestCase() {
    val underTest = EditTileListState(TestEditTiles)
    private val underTest = EditTileListState(TestEditTiles, 4)

    @Test
    fun movingNonExistentTile_tileAdded() {
        val newTile = createEditTile("other_tile", false)
        underTest.move(newTile, TestEditTiles[0].tile.tileSpec)

        assertThat(underTest.tiles[0]).isEqualTo(newTile)
        assertThat(underTest.tiles.subList(1, underTest.tiles.size))
            .containsExactly(*TestEditTiles.toTypedArray())
    fun noDrag_listUnchanged() {
        underTest.tiles.forEach { assertThat(it).isNotInstanceOf(SpacerGridCell::class.java) }
        assertThat(underTest.tiles.map { (it as TileGridCell).tile.tileSpec })
            .containsExactly(*TestEditTiles.map { it.tile.tileSpec }.toTypedArray())
    }

    @Test
    fun movingTileToNonExistentTarget_listUnchanged() {
        underTest.move(TestEditTiles[0], TileSpec.create("other_tile"))
    fun startDrag_listHasSpacers() {
        underTest.onStarted(TestEditTiles[0])

        // [ a ] [ b ] [ c ] [ X ]
        // [ Large D ] [ e ] [ X ]
        assertThat(underTest.tiles.toStrings())
            .isEqualTo(listOf("a", "b", "c", "spacer", "d", "e", "spacer"))
        assertThat(underTest.isMoving(TestEditTiles[0].tile.tileSpec)).isTrue()
        assertThat(underTest.dragInProgress).isTrue()
    }

        assertThat(underTest.tiles).containsExactly(*TestEditTiles.toTypedArray())
    @Test
    fun moveDrag_listChanges() {
        underTest.onStarted(TestEditTiles[4])
        underTest.onMoved(3, false)

        // Tile E goes to index 3
        // [ a ] [ b ] [ c ] [ e ]
        // [ Large D ] [ X ] [ X ]
        assertThat(underTest.tiles.toStrings())
            .isEqualTo(listOf("a", "b", "c", "e", "d", "spacer", "spacer"))
    }

    @Test
    fun movingTileToItself_listUnchanged() {
        underTest.move(TestEditTiles[0], TestEditTiles[0].tile.tileSpec)
    fun moveDragOnSidesOfLargeTile_listChanges() {
        val draggedCell = TestEditTiles[4]

        underTest.onStarted(draggedCell)
        underTest.onMoved(4, true)

        assertThat(underTest.tiles).containsExactly(*TestEditTiles.toTypedArray())
        // Tile E goes to the right side of tile D, list is unchanged
        // [ a ] [ b ] [ c ] [ X ]
        // [ Large D ] [ e ] [ X ]
        assertThat(underTest.tiles.toStrings())
            .isEqualTo(listOf("a", "b", "c", "spacer", "d", "e", "spacer"))

        underTest.onMoved(4, false)

        // Tile E goes to the left side of tile D, they swap positions
        // [ a ] [ b ] [ c ] [ e ]
        // [ Large D ] [ X ] [ X ]
        assertThat(underTest.tiles.toStrings())
            .isEqualTo(listOf("a", "b", "c", "e", "d", "spacer", "spacer"))
    }

    @Test
    fun movingTileToSameSection_listUpdates() {
        // Move tile at index 0 to index 1. Tile 0 should remain current.
        underTest.move(TestEditTiles[0], TestEditTiles[1].tile.tileSpec)
    fun moveNewTile_tileIsAdded() {
        val newTile = createEditTile("newTile", 2)

        underTest.onStarted(newTile)
        underTest.onMoved(5, false)

        // New tile goes to index 5
        // [ a ] [ b ] [ c ] [ X ]
        // [ Large D ] [ newTile ]
        // [ e ] [ X ] [ X ] [ X ]
        assertThat(underTest.tiles.toStrings())
            .isEqualTo(
                listOf("a", "b", "c", "spacer", "d", "newTile", "e", "spacer", "spacer", "spacer")
            )
    }

        // Assert the tiles 0 and 1 have changed places.
        assertThat(underTest.tiles[0]).isEqualTo(TestEditTiles[1])
        assertThat(underTest.tiles[1]).isEqualTo(TestEditTiles[0])
    @Test
    fun droppedNewTile_spacersDisappear() {
        underTest.onStarted(TestEditTiles[0])
        underTest.onDrop()

        // Assert the rest of the list is unchanged
        assertThat(underTest.tiles.subList(2, 5))
            .containsExactly(*TestEditTiles.subList(2, 5).toTypedArray())
        assertThat(underTest.tiles.toStrings()).isEqualTo(listOf("a", "b", "c", "d", "e"))
        assertThat(underTest.isMoving(TestEditTiles[0].tile.tileSpec)).isFalse()
        assertThat(underTest.dragInProgress).isFalse()
    }

    fun removingTile_listUpdates() {
        // Remove tile at index 0
        underTest.remove(TestEditTiles[0].tile.tileSpec)
    @Test
    fun movedTileOutOfBounds_tileDisappears() {
        underTest.onStarted(TestEditTiles[0])
        underTest.movedOutOfBounds()

        // Assert the tile was removed
        assertThat(underTest.tiles).containsExactly(*TestEditTiles.subList(1, 6).toTypedArray())
        assertThat(underTest.tiles.toStrings()).doesNotContain(TestEditTiles[0].tile.tileSpec.spec)
    }

    private fun List<GridCell>.toStrings(): List<String> {
        return map {
            if (it is TileGridCell) {
                it.tile.tileSpec.spec
            } else {
                "spacer"
            }
        }
    }

    companion object {
        private fun createEditTile(
            tileSpec: String,
            isCurrent: Boolean
        ): SizedTile<EditTileViewModel> {
        private fun createEditTile(tileSpec: String, width: Int): SizedTile<EditTileViewModel> {
            return SizedTileImpl(
                EditTileViewModel(
                    tileSpec = TileSpec.create(tileSpec),
                    icon = Icon.Resource(0, null),
                    label = Text.Loaded("unused"),
                    appName = null,
                    isCurrent = isCurrent,
                    isCurrent = true,
                    availableEditActions = emptySet(),
                ),
                1,
                width,
            )
        }

        // [ a ] [ b ] [ c ]
        // [ Large D ] [ e ] [ f ]
        private val TestEditTiles =
            listOf(
                createEditTile("tileA", true),
                createEditTile("tileB", true),
                createEditTile("tileC", true),
                createEditTile("tileD", false),
                createEditTile("tileE", false),
                createEditTile("tileF", false),
                createEditTile("a", 1),
                createEditTile("b", 1),
                createEditTile("c", 1),
                createEditTile("d", 2),
                createEditTile("e", 1),
            )
    }
}
+55 −91

File changed.

Preview size limit exceeded, changes collapsed.

+1 −0
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ fun EditMode(
            Modifier,
            viewModel::addTile,
            viewModel::removeTile,
            viewModel::setTiles,
        )
    }
}
Loading