Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cached onMove lambda causes IndexOutOfBounds crash #290

Open
eschaumloeffel opened this issue Nov 4, 2024 · 0 comments
Open

Cached onMove lambda causes IndexOutOfBounds crash #290

eschaumloeffel opened this issue Nov 4, 2024 · 0 comments

Comments

@eschaumloeffel
Copy link

Hi!

I have a project where there are items in a LazyColumn in a kind of hierarchical directory/file structure.
My problem arises when I try to factor out some of the code into a new composable function and handle the reordering code within that funtion. As soon as I do that, I get IOOB crashes when reordering.

I tried to strip it down to a demo to get my point across:

class MainActivity : ComponentActivity() {

    private val allItems = buildItems()

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        enableEdgeToEdge()

        setContent {
            ReorderableTestTheme {
                var displayItems by remember { mutableStateOf(allItems) }

                Scaffold(modifier = Modifier.fillMaxSize()) { innerPadding ->
                    ListView(
                        items = displayItems,
                        modifier = Modifier.padding(innerPadding),
                        onItemClick = {
                            if(it.children.isEmpty()) {
                                // show item in another screen
                            } else {
                                displayItems = it.children
                            }
                        },
                        onReorder = { from, to ->
                            displayItems = displayItems.toMutableList()
                                .apply { add(to, removeAt(from)) }
                        }
                    )
                }
            }
        }
    }

    private fun buildItems(): List<DisplayItem> {
        return listOf(
            DisplayItem(
                title = "Dir1",
                children = (1..20).map { DisplayItem("Dir1-Item$it") },
            ),
            DisplayItem(
                title = "Dir2",
                children = (1..20).map { DisplayItem("Dir2-Item$it") },
            ),
            DisplayItem("Item1"),
            DisplayItem("Item2"),
            DisplayItem("Item3"),
        )
    }
}

@OptIn(ExperimentalFoundationApi::class)
@Composable
fun ListView(
    items: List<DisplayItem>,
    modifier: Modifier = Modifier,
    onItemClick: (DisplayItem) -> Unit,
    onReorder: (from: Int, to: Int) -> Unit,
) {
    var orderedItems by remember(items) { mutableStateOf(items) }

    val reorderableLazyListState = rememberReorderableLazyListState(
        onMove = { from, to ->
            orderedItems = orderedItems.toMutableList()
                .apply { add(to.index, removeAt(from.index)) }
        },
        onDragEnd = onReorder,
    )

    LazyColumn(
        state = reorderableLazyListState.listState,
        modifier = modifier
            .fillMaxSize()
            .reorderable(reorderableLazyListState)
            .detectReorderAfterLongPress(reorderableLazyListState)
    ) {
        itemsIndexed(orderedItems, { _, it -> it.title }) { index, it ->
            if (index != 0) HorizontalDivider()
            ReorderableItem(
                state = reorderableLazyListState,
                key = { it.title },
                defaultDraggingModifier = Modifier.animateItemPlacement(),
                orientationLocked = true,
                index = index,
            ) { _ ->
                Row(
                    horizontalArrangement = Arrangement.SpaceBetween,
                    verticalAlignment = Alignment.CenterVertically,
                    modifier = Modifier
                        .fillMaxSize()
                        .background(Color.White)
                        .clickable { onItemClick(it) }
                        .animateItemPlacement(),
                ) {
                    Text(it.title, modifier = Modifier.padding(24.dp))
                    if(it.children.isNotEmpty()) {
                        Icon(
                            imageVector = Icons.Default.KeyboardArrowRight,
                            contentDescription = null,
                        )
                    }
                }
            }
        }
    }
}

data class DisplayItem(
    val title: String,
    val children: List<DisplayItem> = emptyList(),
)

In this example, the first items you see are the 5 top level items (Dir1, Dir2, Item1, Item2, Item3). When you click on a directory, the child items are displayed instead. When you now try to reorder an item with index >= 5, you get an IOOB Exception.
My guess, like the title says, is that the onMove lambda is cached and the action is performed on the old list. In other word, even if orderedItems changes, the onMove lambda doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant