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

Commit 7e8462f5 authored by brycelee's avatar brycelee
Browse files

Do not pass indices between drag state and content list state. 

Currently, the position of a items in the lazy grid are passed to the
content state in order to perform operations on the object. This can
lead to inconsistencies as it is not guaranteed these two
representations are kept entirely in sync. For example, a placeholder
might be removed or an object adjusted before an operation may occur.

This changelist addresses this by using the item key as a stable
identifier. This value can be used to look up the position of the item
in each list representation.

Fixes: 417338925
Test: manual - ensured drag and drop operations worked without crashing
Flag: EXEMPT bugfix
Change-Id: Ic6681867066fafa5c12c083d1c700e5ead9c3de5
parent 10c3d590

packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/ContentListState.kt

+4 −2
Original line number Diff line number Diff line
@@ -164,6 +164,8 @@ internal constructor( 
        onReorderWidgets(widgetIdToRankMap)

    }



    /** Returns true if the item at given index is editable. */

    fun isItemEditable(index: Int) = list[index].isWidgetContent()

    /** Returns true if the item for the given key is editable. */

    fun isItemEditable(key: Any): Boolean {

        return list.firstOrNull { content -> content.key == key }?.isWidgetContent() ?: false

    }

}

packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/DragAndDropTargetState.kt

+33 −8
Original line number Diff line number Diff line
@@ -240,7 +240,7 @@ private class DragAndDropTargetStateV1( 
        val targetItem =

            state.layoutInfo.visibleItemsInfo

                .asSequence()

                .filter { item -> contentListState.isItemEditable(item.index) }

                .filter { item -> contentListState.isItemEditable(item.key) }

                .firstItemAtOffset(dragOffset - contentOffset)



        if (
@@ -265,8 +265,8 @@ private class DragAndDropTargetStateV1( 
                scrollOffset = state.firstVisibleItemScrollOffset

            }



            if (contentListState.isItemEditable(targetItem.index)) {

                movePlaceholderTo(targetItem.index)

            if (contentListState.isItemEditable(targetItem.key)) {

                movePlaceholderTo(targetItem.key)

                placeHolderIndex = targetItem.index

            }
@@ -330,8 +330,21 @@ private class DragAndDropTargetStateV1( 
        }

    }



    private fun movePlaceholderTo(index: Int) {

    private fun movePlaceholderTo(key: Any) {

        // This method is often invoked in a separate scope and therefore the placeholder could

        // have disappeared before this point.

        val currentIndex = contentListState.list.indexOf(placeHolder)



        if (currentIndex == -1) {

            return

        }



        val index = contentListState.list.indexOfFirst { item -> item.key == key }



        if (index == -1) {

            return

        }



        if (currentIndex != index) {

            contentListState.onMove(currentIndex, index)

        }
@@ -428,7 +441,7 @@ private class DragAndDropTargetStateV2( 
        val targetItem =

            state.layoutInfo.visibleItemsInfo

                .asSequence()

                .filter { item -> contentListState.isItemEditable(item.index) }

                .filter { item -> contentListState.isItemEditable(item.key) }

                .firstItemAtOffset(dragOffset - contentOffset)



        if (
@@ -456,10 +469,10 @@ private class DragAndDropTargetStateV2( 
            if (scrollToIndex != null) {

                scope.launch {

                    state.scrollToItem(scrollToIndex, state.firstVisibleItemScrollOffset)

                    movePlaceholderTo(targetItem.index)

                    movePlaceholderTo(targetItem.key)

                }

            } else {

                movePlaceholderTo(targetItem.index)

                movePlaceholderTo(targetItem.key)

            }



            placeHolderIndex = targetItem.index
@@ -494,7 +507,19 @@ private class DragAndDropTargetStateV2( 
        }

    }



    private fun movePlaceholderTo(index: Int) {

    private fun movePlaceholderTo(key: Any) {

        // This method is often invoked in a separate scope and therefore the placeholder could

        // have disappeared before this point.

        if (!contentListState.list.contains(placeHolder)) {

            return

        }



        val index = contentListState.list.indexOfFirst { item -> item.key == key }



        if (index == -1) {

            return

        }



        val currentIndex = contentListState.list.indexOf(placeHolder)

        if (currentIndex != index) {

            contentListState.swapItems(currentIndex, index)

packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/GridDragDropState.kt

+6 −6
Original line number Diff line number Diff line
@@ -231,7 +231,7 @@ private class GridDragDropStateV1( 
                offset.y,

            )

        state.layoutInfo.visibleItemsInfo

            .filter { item -> contentListState.isItemEditable(item.index) }

            .filter { item -> contentListState.isItemEditable(item.key) }

            // grid item offset is based off grid content container so we need to deduct

            // before content padding from the initial pointer position

            .firstItemAtOffset(normalizedOffset - contentOffset)
@@ -297,7 +297,7 @@ private class GridDragDropStateV1( 
                    val lastVisibleItemIndex = state.layoutInfo.visibleItemsInfo.last().index

                    val itemBoundingBox = IntRect(item.offset, item.size)

                    draggingItemKey != item.key &&

                        contentListState.isItemEditable(item.index) &&

                        contentListState.isItemEditable(item.key) &&

                        (draggingBoundingBox.contains(itemBoundingBox.center) ||

                            itemBoundingBox.contains(draggingBoundingBox.center)) &&

                        // If we swap with the last visible item, and that item doesn't fit
@@ -310,7 +310,7 @@ private class GridDragDropStateV1( 
            } else {

                state.layoutInfo.visibleItemsInfo

                    .asSequence()

                    .filter { item -> contentListState.isItemEditable(item.index) }

                    .filter { item -> contentListState.isItemEditable(item.key) }

                    .filter { item -> draggingItem.index != item.index }

                    .firstItemAtOffset(middleOffset)

            }
@@ -446,7 +446,7 @@ private class GridDragDropStateV2( 
        this.contentOffset = contentOffset



        state.layoutInfo.visibleItemsInfo

            .filter { item -> contentListState.isItemEditable(item.index) }

            .filter { item -> contentListState.isItemEditable(item.key) }

            // grid item offset is based off grid content container so we need to deduct

            // before content padding from the initial pointer position

            .firstItemAtOffset(normalizedOffset - contentOffset)
@@ -527,7 +527,7 @@ private class GridDragDropStateV2( 
                    fun(item): Boolean {

                        val itemBoundingBox = IntRect(item.offset, item.size)

                        return draggingItemKey != item.key &&

                            contentListState.isItemEditable(item.index) &&

                            contentListState.isItemEditable(item.key) &&

                            itemBoundingBox.contains(curDragPositionInGrid.round()) &&

                            // If we swap with the last visible item, and that item doesn't fit

                            // in the gap created by moving the current item, then the current item
@@ -540,7 +540,7 @@ private class GridDragDropStateV2( 
            } else {

                state.layoutInfo.visibleItemsInfo

                    .asSequence()

                    .filter { item -> contentListState.isItemEditable(item.index) }

                    .filter { item -> contentListState.isItemEditable(item.key) }

                    .filter { item -> draggingItem.index != item.index }

                    .firstItemAtOffset(curDragPositionInGrid)

            }