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

Commit 4a1240b7 authored by Lucas Silva's avatar Lucas Silva
Browse files

Improve drag-to-remove detection

Instead of checking the position of the finger during the drag gesture,
we now check if the bounding box of the item being dragged contains the
center of the remove button. This makes dragging to remove much easier,
especially for larger widgets.

Test: manually by dragging widgets to the remove button
Flag: EXEMPT bugfix
Fixes: 319134429
Change-Id: I42658889e301ed84c4a511fc14abcd332e5271c3
parent ff72c930
Loading
Loading
Loading
Loading
+16 −26
Original line number Original line Diff line number Diff line
@@ -24,7 +24,6 @@ import android.util.SizeF
import android.view.MotionEvent
import android.view.MotionEvent
import android.widget.FrameLayout
import android.widget.FrameLayout
import android.widget.RemoteViews
import android.widget.RemoteViews
import androidx.annotation.VisibleForTesting
import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.core.LinearEasing
import androidx.compose.animation.core.LinearEasing
import androidx.compose.animation.core.Spring
import androidx.compose.animation.core.Spring
@@ -149,9 +148,11 @@ import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.IntRect
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.round
import androidx.compose.ui.unit.sp
import androidx.compose.ui.unit.sp
import androidx.compose.ui.unit.times
import androidx.compose.ui.unit.times
import androidx.compose.ui.util.fastAll
import androidx.compose.ui.util.fastAll
@@ -386,13 +387,19 @@ fun CommunalHub(
                            contentOffset = contentOffset,
                            contentOffset = contentOffset,
                            screenWidth = screenWidth,
                            screenWidth = screenWidth,
                            setGridCoordinates = { gridCoordinates = it },
                            setGridCoordinates = { gridCoordinates = it },
                            updateDragPositionForRemove = { offset ->
                            updateDragPositionForRemove = { boundingBox ->
                                isPointerWithinEnabledRemoveButton(
                                val gridOffset = gridCoordinates?.positionInWindow()
                                    removeEnabled = removeButtonEnabled,
                                val removeButtonCenter =
                                    offset =
                                    removeButtonCoordinates?.boundsInWindow()?.center
                                        gridCoordinates?.let { it.positionInWindow() + offset },
                                removeButtonEnabled &&
                                    containerToCheck = removeButtonCoordinates,
                                    gridOffset != null &&
                                )
                                    removeButtonCenter != null &&
                                    boundingBox
                                        // The bounding box is relative to the grid, so we need to
                                        // normalize it by adding the grid offset and the content
                                        // offset.
                                        .translate((gridOffset + contentOffset).round())
                                        .contains(removeButtonCenter.round())
                            },
                            },
                            gridState = gridState,
                            gridState = gridState,
                            contentListState = contentListState,
                            contentListState = contentListState,
@@ -685,7 +692,7 @@ private fun BoxScope.CommunalHubLazyGrid(
    gridState: LazyGridState,
    gridState: LazyGridState,
    contentListState: ContentListState,
    contentListState: ContentListState,
    setGridCoordinates: (coordinates: LayoutCoordinates) -> Unit,
    setGridCoordinates: (coordinates: LayoutCoordinates) -> Unit,
    updateDragPositionForRemove: (offset: Offset) -> Boolean,
    updateDragPositionForRemove: (boundingBox: IntRect) -> Boolean,
    widgetConfigurator: WidgetConfigurator?,
    widgetConfigurator: WidgetConfigurator?,
    interactionHandler: RemoteViews.InteractionHandler?,
    interactionHandler: RemoteViews.InteractionHandler?,
    widgetSection: CommunalAppWidgetSection,
    widgetSection: CommunalAppWidgetSection,
@@ -1556,23 +1563,6 @@ private fun beforeContentPadding(paddingValues: PaddingValues): ContentPaddingIn
    }
    }
}
}


/**
 * Check whether the pointer position that the item is being dragged at is within the coordinates of
 * the remove button in the toolbar. Returns true if the item is removable.
 */
@VisibleForTesting
fun isPointerWithinEnabledRemoveButton(
    removeEnabled: Boolean,
    offset: Offset?,
    containerToCheck: LayoutCoordinates?,
): Boolean {
    if (!removeEnabled || offset == null || containerToCheck == null) {
        return false
    }
    val container = containerToCheck.boundsInWindow()
    return container.contains(offset)
}

private fun CommunalContentSize.dp(): Dp {
private fun CommunalContentSize.dp(): Dp {
    return when (this) {
    return when (this) {
        CommunalContentSize.FULL -> Dimensions.CardHeightFull
        CommunalContentSize.FULL -> Dimensions.CardHeightFull
+11 −12
Original line number Original line Diff line number Diff line
@@ -58,11 +58,11 @@ private fun Float.directional(origin: LayoutDirection, current: LayoutDirection)
fun rememberGridDragDropState(
fun rememberGridDragDropState(
    gridState: LazyGridState,
    gridState: LazyGridState,
    contentListState: ContentListState,
    contentListState: ContentListState,
    updateDragPositionForRemove: (offset: Offset) -> Boolean,
    updateDragPositionForRemove: (boundingBox: IntRect) -> Boolean,
): GridDragDropState {
): GridDragDropState {
    val scope = rememberCoroutineScope()
    val scope = rememberCoroutineScope()
    val state =
    val state =
        remember(gridState, contentListState) {
        remember(gridState, contentListState, updateDragPositionForRemove) {
            GridDragDropState(
            GridDragDropState(
                state = gridState,
                state = gridState,
                contentListState = contentListState,
                contentListState = contentListState,
@@ -92,7 +92,7 @@ internal constructor(
    private val state: LazyGridState,
    private val state: LazyGridState,
    private val contentListState: ContentListState,
    private val contentListState: ContentListState,
    private val scope: CoroutineScope,
    private val scope: CoroutineScope,
    private val updateDragPositionForRemove: (offset: Offset) -> Boolean,
    private val updateDragPositionForRemove: (draggingBoundingBox: IntRect) -> Boolean,
) {
) {
    var draggingItemKey by mutableStateOf<Any?>(null)
    var draggingItemKey by mutableStateOf<Any?>(null)
        private set
        private set
@@ -104,7 +104,6 @@ internal constructor(


    private var draggingItemDraggedDelta by mutableStateOf(Offset.Zero)
    private var draggingItemDraggedDelta by mutableStateOf(Offset.Zero)
    private var draggingItemInitialOffset by mutableStateOf(Offset.Zero)
    private var draggingItemInitialOffset by mutableStateOf(Offset.Zero)
    private var dragStartPointerOffset by mutableStateOf(Offset.Zero)


    private var previousTargetItemKey: Any? = null
    private var previousTargetItemKey: Any? = null


@@ -139,7 +138,6 @@ internal constructor(
            // before content padding from the initial pointer position
            // before content padding from the initial pointer position
            .firstItemAtOffset(normalizedOffset - contentOffset)
            .firstItemAtOffset(normalizedOffset - contentOffset)
            ?.apply {
            ?.apply {
                dragStartPointerOffset = normalizedOffset - this.offset.toOffset()
                draggingItemKey = key
                draggingItemKey = key
                draggingItemInitialOffset = this.offset.toOffset()
                draggingItemInitialOffset = this.offset.toOffset()
                return true
                return true
@@ -155,7 +153,7 @@ internal constructor(
                    contentListState.list.indexOfFirst { it.key == draggingItemKey }
                    contentListState.list.indexOfFirst { it.key == draggingItemKey }
                )
                )
                isDraggingToRemove = false
                isDraggingToRemove = false
                updateDragPositionForRemove(Offset.Zero)
                updateDragPositionForRemove(IntRect.Zero)
            }
            }
            // persist list editing changes on dragging ends
            // persist list editing changes on dragging ends
            contentListState.onSaveList()
            contentListState.onSaveList()
@@ -164,7 +162,6 @@ internal constructor(
        previousTargetItemKey = null
        previousTargetItemKey = null
        draggingItemDraggedDelta = Offset.Zero
        draggingItemDraggedDelta = Offset.Zero
        draggingItemInitialOffset = Offset.Zero
        draggingItemInitialOffset = Offset.Zero
        dragStartPointerOffset = Offset.Zero
    }
    }


    internal fun onDrag(offset: Offset, layoutDirection: LayoutDirection) {
    internal fun onDrag(offset: Offset, layoutDirection: LayoutDirection) {
@@ -230,7 +227,7 @@ internal constructor(
            if (overscroll != 0f) {
            if (overscroll != 0f) {
                scrollChannel.trySend(overscroll)
                scrollChannel.trySend(overscroll)
            }
            }
            isDraggingToRemove = checkForRemove(startOffset)
            isDraggingToRemove = checkForRemove(draggingBoundingBox)
            previousTargetItemKey = null
            previousTargetItemKey = null
        }
        }
    }
    }
@@ -247,10 +244,12 @@ internal constructor(
    }
    }


    /** Calls the callback with the updated drag position and returns whether to remove the item. */
    /** Calls the callback with the updated drag position and returns whether to remove the item. */
    private fun checkForRemove(startOffset: Offset): Boolean {
    private fun checkForRemove(draggingItemBoundingBox: IntRect): Boolean {
        return if (draggingItemDraggedDelta.y < 0)
        return if (draggingItemDraggedDelta.y < 0) {
            updateDragPositionForRemove(startOffset + dragStartPointerOffset)
            updateDragPositionForRemove(draggingItemBoundingBox)
        else false
        } else {
            false
        }
    }
    }
}
}


+0 −41
Original line number Original line 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.communal.ui.compose

import android.testing.TestableLooper
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.layout.LayoutCoordinates
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.mock

@RunWith(AndroidJUnit4::class)
@TestableLooper.RunWithLooper(setAsMainLooper = true)
@SmallTest
class CommunalHubUtilsTest : SysuiTestCase() {
    @Test
    fun isPointerWithinEnabledRemoveButton_ensureDisabledStatePriority() {
        assertThat(
                isPointerWithinEnabledRemoveButton(false, mock<Offset>(), mock<LayoutCoordinates>())
            )
            .isFalse()
    }
}