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

Commit e7e595de authored by Lucas Silva's avatar Lucas Silva
Browse files

Address some issues with ResizeableItemFrame 

1. Use rememberUpdatedState to remember the resize callback, and ensure
   we always use the latest one in the event of recomposition.
2. Make sure we clear anchors when the layout info becomes unknown, such
   as when the item is scrolled out-of-view.
3. Instead of using the index, use a key to locate items. The index may
   change if the item is moved as part of resizing, which will cause a
   recomposition. However, the key should remain stable.

Bug: 368056517
Test: atest ResizeableItemFrameViewModelTest
Flag: EXEMPT component is not yet used anywhere
Change-Id: I17d4fc1180866d18bfb61f8de739e794e80da886
parent 4c3a7c43

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

+14 −11
Original line number Diff line number Diff line
@@ -30,6 +30,8 @@ import androidx.compose.foundation.layout.height 
import androidx.compose.foundation.lazy.grid.LazyGridState

import androidx.compose.runtime.Composable

import androidx.compose.runtime.LaunchedEffect

import androidx.compose.runtime.getValue

import androidx.compose.runtime.rememberUpdatedState

import androidx.compose.runtime.snapshotFlow

import androidx.compose.ui.Alignment

import androidx.compose.ui.Modifier
@@ -52,12 +54,11 @@ import com.android.systemui.communal.ui.viewmodel.ResizeableItemFrameViewModel 
import com.android.systemui.lifecycle.rememberViewModel

import kotlinx.coroutines.flow.collectLatest

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.filterNotNull



@Composable

private fun UpdateGridLayoutInfo(

    viewModel: ResizeableItemFrameViewModel,

    index: Int,

    key: String,

    gridState: LazyGridState,

    minItemSpan: Int,

    gridContentPadding: PaddingValues,
@@ -67,7 +68,7 @@ private fun UpdateGridLayoutInfo( 
    LaunchedEffect(

        density,

        viewModel,

        index,

        key,

        gridState,

        minItemSpan,

        gridContentPadding,
@@ -85,9 +86,8 @@ private fun UpdateGridLayoutInfo( 
                snapshotFlow { gridState.layoutInfo.maxSpan },

                snapshotFlow { gridState.layoutInfo.viewportSize.height },

                snapshotFlow {

                        gridState.layoutInfo.visibleItemsInfo.firstOrNull { it.index == index }

                    }

                    .filterNotNull(),

                    gridState.layoutInfo.visibleItemsInfo.firstOrNull { it.key == key }

                },

                ::Triple,

            )

            .collectLatest { (maxItemSpan, viewportHeightPx, itemInfo) ->
@@ -97,8 +97,8 @@ private fun UpdateGridLayoutInfo( 
                    viewportHeightPx,

                    maxItemSpan,

                    minItemSpan,

                    itemInfo.row,

                    itemInfo.span,

                    itemInfo?.row,

                    itemInfo?.span,

                )

            }

    }
@@ -161,7 +161,7 @@ private fun BoxScope.DragHandle( 
 */

@Composable

fun ResizableItemFrame(

    index: Int,

    key: String,

    gridState: LazyGridState,

    minItemSpan: Int,

    gridContentPadding: PaddingValues,
@@ -177,6 +177,7 @@ fun ResizableItemFrame( 
    content: @Composable () -> Unit,

) {

    val brush = SolidColor(outlineColor)

    val onResizeUpdated by rememberUpdatedState(onResize)

    val viewModel =

        rememberViewModel(traceName = "ResizeableItemFrame.viewModel") {

            ResizeableItemFrameViewModel()
@@ -230,13 +231,15 @@ fun ResizableItemFrame( 


            UpdateGridLayoutInfo(

                viewModel,

                index,

                key,

                gridState,

                minItemSpan,

                gridContentPadding,

                verticalArrangement,

            )

            LaunchedEffect(viewModel) { viewModel.resizeInfo.collectLatest(onResize) }

            LaunchedEffect(viewModel) {

                viewModel.resizeInfo.collectLatest { info -> onResizeUpdated(info) }

            }

        }

    }

}

packages/SystemUI/multivalentTests/src/com/android/systemui/communal/ui/viewmodel/ResizeableItemFrameViewModelTest.kt

+19 −2
Original line number Diff line number Diff line
@@ -254,6 +254,23 @@ class ResizeableItemFrameViewModelTest : SysuiTestCase() { 
        assertThat(resizeInfo).isEqualTo(ResizeInfo(-1, DragHandle.BOTTOM))

    }



    @Test

    fun testRowInfoBecomesNull_revertsBackToDefault() =

        testScope.runTest {

            val gridLayout = singleSpanGrid.copy(maxItemSpan = 3, currentRow = 1)

            updateGridLayout(gridLayout)



            val topState = underTest.topDragState

            assertThat(topState.anchors.toList()).containsExactly(0 to 0f, -1 to -30f)



            val bottomState = underTest.bottomDragState

            assertThat(bottomState.anchors.toList()).containsExactly(0 to 0f, 1 to 30f)



            updateGridLayout(gridLayout.copy(currentRow = null))

            assertThat(topState.anchors.toList()).containsExactly(0 to 0f)

            assertThat(bottomState.anchors.toList()).containsExactly(0 to 0f)

        }



    @Test(expected = IllegalArgumentException::class)

    fun testIllegalState_maxSpanSmallerThanMinSpan() =

        testScope.runTest {
@@ -317,7 +334,7 @@ class ResizeableItemFrameViewModelTest : SysuiTestCase() { 
        val viewportHeightPx: Int,

        val maxItemSpan: Int,

        val minItemSpan: Int,

        val currentRow: Int,

        val currentSpan: Int,

        val currentRow: Int?,

        val currentSpan: Int?,

    )

}

packages/SystemUI/src/com/android/systemui/communal/ui/viewmodel/ResizeableItemFrameViewModel.kt

+14 −9
Original line number Diff line number Diff line
@@ -25,8 +25,7 @@ import kotlinx.coroutines.awaitCancellation 
import kotlinx.coroutines.flow.Flow

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.distinctUntilChanged

import kotlinx.coroutines.flow.dropWhile

import kotlinx.coroutines.flow.filterNotNull

import kotlinx.coroutines.flow.filter

import kotlinx.coroutines.flow.launchIn

import kotlinx.coroutines.flow.map

import kotlinx.coroutines.flow.merge
@@ -45,7 +44,10 @@ data class ResizeInfo( 
    val spans: Int,

    /** The drag handle which was used to resize the element. */

    val fromHandle: DragHandle,

)

) {

    /** Whether we are expanding. If false, then we are shrinking. */

    val isExpanding = spans > 0

}



class ResizeableItemFrameViewModel : ExclusiveActivatable() {

    private data class GridLayoutInfo(
@@ -73,7 +75,7 @@ class ResizeableItemFrameViewModel : ExclusiveActivatable() { 
                snapshotFlow { bottomDragState.settledValue }

                    .map { ResizeInfo(it, DragHandle.BOTTOM) },

            )

            .dropWhile { it.spans == 0 }

            .filter { it.spans != 0 }

            .distinctUntilChanged()



    /**
@@ -86,9 +88,13 @@ class ResizeableItemFrameViewModel : ExclusiveActivatable() { 
        viewportHeightPx: Int,

        maxItemSpan: Int,

        minItemSpan: Int,

        currentRow: Int,

        currentSpan: Int,

        currentRow: Int?,

        currentSpan: Int?,

    ) {

        if (currentSpan == null || currentRow == null) {

            gridLayoutInfo.value = null

            return

        }

        require(maxItemSpan >= minItemSpan) {

            "Maximum item span of $maxItemSpan cannot be less than the minimum span of $minItemSpan"

        }
@@ -114,10 +120,10 @@ class ResizeableItemFrameViewModel : ExclusiveActivatable() { 


    private fun calculateAnchorsForHandle(

        handle: DragHandle,

        layoutInfo: GridLayoutInfo,

        layoutInfo: GridLayoutInfo?,

    ): DraggableAnchors<Int> {



        if (!isDragAllowed(handle, layoutInfo)) {

        if (layoutInfo == null || !isDragAllowed(handle, layoutInfo)) {

            return DraggableAnchors { 0 at 0f }

        }
@@ -188,7 +194,6 @@ class ResizeableItemFrameViewModel : ExclusiveActivatable() { 
    override suspend fun onActivated(): Nothing {

        coroutineScope("ResizeableItemFrameViewModel.onActivated") {

            gridLayoutInfo

                .filterNotNull()

                .onEach { layoutInfo ->

                    topDragState.updateAnchors(

                        calculateAnchorsForHandle(DragHandle.TOP, layoutInfo)