android-jetpack-compose

Problem with deleting object in jetpack compose


I have this screen in which I create 5 draggable objects. Everything should be working as expected. You can drag them and place them everywhere on the screen. Here is my MainActivity:

class MainActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            GraggableItemsTheme {

                val viewModel: MainViewModel by viewModels()

                val list = viewModel.scoreData.collectAsState()
                // A surface container using the 'background' color from the theme
                Surface(
                    modifier = Modifier.fillMaxSize(),
                    color = MaterialTheme.colorScheme.background
                ) {
                    list.value?.forEach {  i ->
                        DraggableTextLowLevel(
                            id = i,
                            onDelete = viewModel::deleteItem
                        )
                    }

                }
            }
        }
    }
}

@Composable
private fun DraggableTextLowLevel(
    id: Int,
    onDelete: (Int) -> Unit
) {
    Box(modifier = Modifier.fillMaxSize()) {
        var offsetX by remember { mutableStateOf(0f) }
        var offsetY by remember { mutableStateOf(0f) }

        Box(
            Modifier
                .offset {
                    IntOffset(
                        offsetX.roundToInt(),
                        offsetY.roundToInt()
                    )
                }
                .background(Color.Blue)
                .size(50.dp)
                .pointerInput(Unit) {
                    detectTapGestures(
                        onTap = {
                            onDelete(id)
                        }
                    )
                }
                .pointerInput(Unit) {
                    detectDragGestures { change, dragAmount ->
                        change.consume()
                        offsetX += dragAmount.x
                        offsetY += dragAmount.y
                    }
                }
        ) {
            Text(text = "$id")
        }
    }
}

and the viewModel:

class MainViewModel : ViewModel() {

private val _scoreData = MutableStateFlow<List<Item>?>(
    listOf(
        Item(10,"one"),
        Item(20,"two"),
        Item(30,"three"),
        Item(40,"four"),
        Item(50,"five")
    )
)
val scoreData: StateFlow<List<Item>?> =
    _scoreData.asStateFlow()

fun deleteItem(number: Int) {
    println(number.toString())
    _scoreData.value = _scoreData.value?.toMutableStateList().also {
        println("Item to delete $number")
        val itemToDelete = it?.find { item ->
            item.id == number
        }
        try {
            it?.remove(itemToDelete)
            println("success")
        }
        catch (e: Exception) {
            println(e.toString())
        }

    }
}

and the data class of item:

data class Item(
    val id: Int = 0,
    val name: String
)

The problem is that when I click an item to delete ui deletes wrong item. I have tried everything I know but no result. Sometimes it erases the expected item but most of the cases it deletes the wrong.


Solution

  • This is because something sneaky and hard to come by.

    This has the same systemic problem all for loops have in that the items are used in order of composition so if data moves around in the collections a lot more recomposition is performed than is strictly necessary. For example, if one item was inserted at the beginning of the collection the entire view would need to be recomposed instead of just the first item being created and the rest being reused unmodified. This can cause the UI to become confused if, for example, input fields with selection are used in the item block as the selection will not track with the data value but with the index order in the collection. If the user selected text in the first item, inserting a new item will cause selection to appear in the new item selecting what appears to be random text and the selection will be lost in the item the user might have expected to still have selection.

    Basically, when you change item positions their state don't match their positions and Compose gets confused because it keeps nodes based on position instead of data.

    The simplest way to solve this is to add key that maps to data.

    val viewModel: MainViewModel by viewModels()
    
    val list = viewModel.scoreData.collectAsState()
    // A surface container using the 'background' color from the theme
    Surface(
        modifier = Modifier.fillMaxSize(),
    ) {
        list.value?.forEach { item ->
            key(
                item.id
            ) {
                DraggableTextLowLevel(
                    id = item.id,
                    onDelete = viewModel::deleteItem
                )
            }
        }
    }
    

    Your code also can be simplified to have one SnapshotStateList that you add items when you collect flow.

    Another way to solve this to not create Composables with state whenever possible. If you hoist state the latest values will passed as parameters.

    Edit

    And less known but cool way to store states of Composable even if they exit composition is to use movableContentOf.

    @Composable
    private fun StatefulCheckBox(title: String, isChecked: Boolean) {
        Row(
            verticalAlignment = Alignment.CenterVertically
        ) {
    
            var checked by remember {
                mutableStateOf(isChecked)
            }
            Text(text = title)
            Checkbox(checked = checked,
                onCheckedChange = {
                    checked = it
                }
            )
        }
    }
    
    
    data class Item(val title: String, val checked: Boolean)
    

    Removing items by default results wrong state in latest index as can be seen in gif. By using movableStateOf state is preserved.

    enter image description here

    @Composable
    private fun StatefulListSample() {
        val list = remember {
            mutableStateListOf(
                Item("Item1", checked = false),
                Item("Item2", checked = true),
                Item("Item3", checked = true),
            )
        }
    
        Button(onClick = {
            list.removeFirstOrNull()
        }) {
            Text(text = "Delete First Item")
        }
    
        list.forEach { item ->
            StatefulCheckBox(item.title, item.checked)
        }
    }
    
    @Composable
    private fun MovableContentOfSample() {
        val list = remember {
            mutableStateListOf(
                Item("Item1", checked = false),
                Item("Item2", checked = true),
                Item("Item3", checked = true),
            )
        }
    
        val movableItems =
            list.map { item ->
                movableContentOf {
                    StatefulCheckBox(item.title, item.checked)
                }
            }
    
        Button(onClick = {
            list.removeFirstOrNull()
        }) {
            Text(text = "Delete First Item")
        }
    
        list.forEachIndexed { index, item ->
            movableItems[index]()
        }
    }
    

    Sources

    https://newsletter.jorgecastillo.dev/p/movablecontentof-and-movablecontentwithreceivero

    https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/runtime/design/movable-content.md