diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 3a7aaecdb..36a8a0c4a 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -55,7 +55,9 @@ configure { System.getProperty("versionNameSuffix")?.let { versionNameSuffix = it } testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" + // https://blog.grobox.de/2019/disable-google-android-instrumentation-test-tracking/ testInstrumentationRunnerArguments["disableAnalytics"] = "true" + // https://developer.android.com/studio/test/espresso-api#set_up_your_project_for_the_espresso_device_api testOptions { emulatorControl { enable = true diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorTest.kt index 190bc6861..adebf3cdc 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorTest.kt @@ -261,7 +261,7 @@ class LongPressMenuEditorTest { assertActionEnabledStatus(label = Enqueue.label, expectedEnabled = true) assertActionEnabledStatus(label = EnqueueNext.label, expectedEnabled = true) assertActionEnabledStatus(label = R.string.long_press_menu_header, expectedEnabled = true) - closeEditorAndAssertNewSettings(isHeaderEnabled = true, actionArrangement = getDefaultEnabledLongPressActions(ctx)) + closeEditorAndAssertNewSettings(isHeaderEnabled = true, actionArrangement = getDefaultLongPressActionArrangement(ctx)) } @Test diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettingsTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettingsTest.kt index 8ebb78993..15706de71 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettingsTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettingsTest.kt @@ -44,13 +44,13 @@ class LongPressMenuSettingsTest { @Test fun testLoadingActionArrangementUnset() { clearPrefs() - assertEquals(getDefaultEnabledLongPressActions(ctx), loadLongPressActionArrangementFromSettings(ctx)) + assertEquals(getDefaultLongPressActionArrangement(ctx), loadLongPressActionArrangementFromSettings(ctx)) } @Test fun testLoadingActionArrangementInvalid() { putStringInPrefs(R.string.long_press_menu_action_arrangement_key, "0,1,whatever,3") - assertEquals(getDefaultEnabledLongPressActions(ctx), loadLongPressActionArrangementFromSettings(ctx)) + assertEquals(getDefaultLongPressActionArrangement(ctx), loadLongPressActionArrangementFromSettings(ctx)) } @Test @@ -73,7 +73,7 @@ class LongPressMenuSettingsTest { fun testDefaultActionsIncludeKodiIffShowKodiEnabled() { for (enabled in arrayOf(false, true)) { putBooleanInPrefs(R.string.show_play_with_kodi_key, enabled) - val actions = getDefaultEnabledLongPressActions(ctx) + val actions = getDefaultLongPressActionArrangement(ctx) assertEquals(enabled, actions.contains(PlayWithKodi)) } } @@ -85,7 +85,7 @@ class LongPressMenuSettingsTest { for (actions in listOf(listOf(Enqueue), listOf(Enqueue, PlayWithKodi))) { storeLongPressActionArrangementToSettings(ctx, actions) addOrRemoveKodiLongPressAction(ctx) - val newActions = getDefaultEnabledLongPressActions(ctx) + val newActions = getDefaultLongPressActionArrangement(ctx) assertEquals(enabled, newActions.contains(PlayWithKodi)) } } diff --git a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuTest.kt b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuTest.kt index b0153cdcc..fbf2c491e 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/ui/components/menu/LongPressMenuTest.kt @@ -21,12 +21,10 @@ import androidx.compose.ui.test.isNotDisplayed import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onFirst -import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.unit.dp -import androidx.test.espresso.Espresso import androidx.test.espresso.Espresso.onView import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches @@ -508,8 +506,8 @@ class LongPressMenuTest { fun testOnlyAndAllArrangedActionsDisplayed3() { assertOnlyAndAllArrangedActionsDisplayed( availableActions = LongPressAction.Type.entries, - actionArrangement = getDefaultEnabledLongPressActions(ctx), - expectedShownActions = getDefaultEnabledLongPressActions(ctx) + actionArrangement = getDefaultLongPressActionArrangement(ctx), + expectedShownActions = getDefaultLongPressActionArrangement(ctx) ) } @@ -534,9 +532,9 @@ class LongPressMenuTest { @Test fun testOnlyAndAllAvailableActionsDisplayed3() { assertOnlyAndAllArrangedActionsDisplayed( - availableActions = getDefaultEnabledLongPressActions(ctx), + availableActions = getDefaultLongPressActionArrangement(ctx), actionArrangement = LongPressAction.Type.entries, - expectedShownActions = getDefaultEnabledLongPressActions(ctx) + expectedShownActions = getDefaultLongPressActionArrangement(ctx) ) } diff --git a/app/src/main/java/org/schabi/newpipe/ui/GestureModifiers.kt b/app/src/main/java/org/schabi/newpipe/ui/GestureModifiers.kt index 2fe995823..d5bfd3634 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/GestureModifiers.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/GestureModifiers.kt @@ -67,15 +67,17 @@ fun Modifier.detectDragGestures( return@withTimeout false } } catch (_: PointerEventTimeoutCancellationException) { - true + true // the timeout fired, so the "press" is indeed "long" } val pointerId = down.id + // importantly, tell `beginDragGesture` whether the drag begun with a long press beginDragGesture(down.position.toIntOffset(), wasLongPressed) while (true) { + // go through all events of this gesture and feed them to `handleDragGestureChange` val change = awaitPointerEvent().changes.find { it.id == pointerId } if (change == null || !change.pressed) { - break + break // the gesture finished } handleDragGestureChange( change.position.toIntOffset(), @@ -97,7 +99,7 @@ private fun Offset.toIntOffset() = IntOffset(this.x.toInt(), this.y.toInt()) fun Modifier.discardAllTouchesIf(doDiscard: Boolean) = if (doDiscard) { pointerInput(Unit) { awaitPointerEventScope { - // we should wait for all new pointer events + // we should wait for all new pointer events and ignore them all while (true) { awaitPointerEvent(pass = PointerEventPass.Initial) .changes diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenu.kt b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenu.kt index d760e52af..5ee0ad0b1 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenu.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenu.kt @@ -832,11 +832,7 @@ private fun LongPressMenuPreview( onDispose {} } - // the incorrect theme is set when running the preview in an emulator for some reason... - val initialUseDarkTheme = isSystemInDarkTheme() - var useDarkTheme by remember { mutableStateOf(initialUseDarkTheme) } - - AppTheme(useDarkTheme = useDarkTheme) { + AppTheme { Surface(color = MaterialTheme.colorScheme.surfaceContainerLow) { // longPressable is null when running the preview in an emulator for some reason... @Suppress("USELESS_ELVIS") diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditor.kt b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditor.kt index a0c898185..ac348d02c 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditor.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditor.kt @@ -1,17 +1,7 @@ /* - * Copyright (C) 2022-2025 The FlorisBoard Contributors - * - * 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. + * SPDX-FileCopyrightText: 2022-2025 The FlorisBoard Contributors + * SPDX-FileCopyrightText: 2026 NewPipe e.V. + * SPDX-License-Identifier: Apache-2.0 AND GPL-3.0-or-later */ @file:OptIn(ExperimentalMaterial3Api::class) @@ -82,6 +72,10 @@ import org.schabi.newpipe.ui.theme.AppTheme import org.schabi.newpipe.util.text.FixedHeightCenteredText /** + * An editor for the actions shown in the [LongPressMenu], that also allows enabling or disabling + * the header. It allows the user to arrange the actions in any way, and to disable them by dragging + * them to a disabled section. + * * When making changes to this composable and to [LongPressMenuEditorState], make sure to test the * following use cases, and check that they still work: * - both the actions and the header can be dragged around @@ -94,7 +88,8 @@ import org.schabi.newpipe.util.text.FixedHeightCenteredText * offset to ensure the user can see the thing being dragged under their finger) * - when the view does not fit the page, it is possible to scroll without moving any item, and * dragging an item towards the top/bottom of the page scrolls up/down - * @author This composable was originally copied from FlorisBoard. + * + * @author This composable was originally copied from FlorisBoard, but was modified significantly. */ @Composable fun LongPressMenuEditorPage(onBackClick: () -> Unit) { @@ -107,6 +102,7 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { DisposableEffect(Unit) { onDispose { + // saves to settings the action arrangement and whether the header is enabled state.onDispose(context) } } @@ -118,7 +114,8 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { ResetToDefaultsButton { state.resetToDefaults(context) } } ) { paddingValues -> - // test scrolling on Android TV by adding `.padding(horizontal = 350.dp)` here + // if you want to forcefully "make the screen smaller" to test scrolling on Android TVs with + // DPAD, add `.padding(horizontal = 350.dp)` here BoxWithConstraints(Modifier.padding(paddingValues)) { // otherwise we wouldn't know the amount of columns to handle the Up/Down key events val columns = maxOf(1, floor(this.maxWidth / MinButtonWidth).toInt()) @@ -126,10 +123,11 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { LazyVerticalGrid( modifier = Modifier .safeDrawingPadding() + // `.detectDragGestures()` handles touch gestures on phones/tablets .detectDragGestures( - beginDragGesture = state::beginDragGesture, - handleDragGestureChange = state::handleDragGestureChange, - endDragGesture = state::completeDragGestureAndCleanUp + beginDragGesture = state::beginDragTouch, + handleDragGestureChange = state::handleDragChangeTouch, + endDragGesture = state::completeDragAndCleanUp ) // `.focusTarget().onKeyEvent()` handles DPAD on Android TVs .focusTarget() @@ -137,6 +135,9 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { .testTag("LongPressMenuEditorGrid"), // same width as the LongPressMenu columns = GridCells.Adaptive(MinButtonWidth), + // Scrolling is handled manually through `.detectDragGestures` above: if the user + // long-presses an item and then moves the finger, the item itself moves; otherwise, + // if the click is too short or the user didn't click on an item, the view scrolls. userScrollEnabled = false, state = gridState ) { @@ -147,25 +148,26 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { ) { i, item -> ItemInListUi( item = item, - selected = state.currentlyFocusedItem == i, + focused = state.currentlyFocusedItem == i, // We only want placement animations: fade in/out animations interfere with // items being replaced by a drag marker while being dragged around, and a // fade in/out animation there does not make sense as the item was just - // "picked up". Furthermore there are strange moving animation artifacts + // "picked up". Furthermore there were strange moving animation artifacts // when moving and releasing items quickly before their fade-out animation - // finishes. + // finishes, so it looks much more polished without fade in/out animations. modifier = Modifier.animateItem(fadeInSpec = null, fadeOutSpec = null) ) } } state.activeDragItem?.let { activeDragItem -> - // draw it the same size as the selected item, + // draw it the same size as the selected item, so it properly appears that the user + // picked up the item and is controlling it with their finger val size = with(LocalDensity.current) { remember(state.activeDragSize) { state.activeDragSize.toSize().toDpSize() } } ItemInListUi( item = activeDragItem, - selected = true, + focused = true, modifier = Modifier .size(size) .offset { state.activeDragPosition } @@ -177,8 +179,12 @@ fun LongPressMenuEditorPage(onBackClick: () -> Unit) { } } +/** + * A button that when clicked opens a confirmation dialog, and then calls [doReset] to reset the + * actions arrangement and whether the header is enabled to their default values. + */ @Composable -private fun ResetToDefaultsButton(onClick: () -> Unit) { +private fun ResetToDefaultsButton(doReset: () -> Unit) { var showDialog by rememberSaveable { mutableStateOf(false) } if (showDialog) { @@ -187,7 +193,7 @@ private fun ResetToDefaultsButton(onClick: () -> Unit) { text = { Text(stringResource(R.string.long_press_menu_reset_to_defaults_confirm)) }, confirmButton = { TextButton(onClick = { - onClick() + doReset() showDialog = false }) { Text(stringResource(R.string.ok)) @@ -208,9 +214,13 @@ private fun ResetToDefaultsButton(onClick: () -> Unit) { ) } +/** + * Renders either [ItemInList.EnabledCaption] or [ItemInList.HiddenCaption], i.e. the full-width + * captions separating enabled and hidden items in the list. + */ @Composable -private fun Subheader( - selected: Boolean, +private fun Caption( + focused: Boolean, @StringRes title: Int, @StringRes description: Int, modifier: Modifier = Modifier @@ -219,7 +229,7 @@ private fun Subheader( modifier = modifier .fillMaxWidth() .padding(horizontal = 16.dp, vertical = 12.dp) - .letIf(selected) { border(2.dp, LocalContentColor.current) } + .letIf(focused) { border(2.dp, LocalContentColor.current) } ) { Text( text = stringResource(title), @@ -233,9 +243,13 @@ private fun Subheader( } } +/** + * Renders all [ItemInList] except captions, that is, all items using a slot of the grid (or two + * horizontal slots in case of the header). + */ @Composable private fun ActionOrHeaderBox( - selected: Boolean, + focused: Boolean, icon: ImageVector, @StringRes text: Int, contentColor: Color, @@ -247,7 +261,7 @@ private fun ActionOrHeaderBox( color = backgroundColor, contentColor = contentColor, shape = MaterialTheme.shapes.large, - border = BorderStroke(2.dp, contentColor.copy(alpha = 1f)).takeIf { selected }, + border = BorderStroke(2.dp, contentColor.copy(alpha = 1f)).takeIf { focused }, modifier = modifier.padding( horizontal = horizontalPadding, vertical = 5.dp @@ -268,26 +282,32 @@ private fun ActionOrHeaderBox( } } +/** + * @param item the [ItemInList] to render using either [Caption] or [ActionOrHeaderBox] with + * different parameters + * @param focused if `true`, a box will be drawn around the item to indicate that it is focused + * (this will only ever be `true` when the user is navigating with DPAD, e.g. on Android TVs) + */ @Composable private fun ItemInListUi( item: ItemInList, - selected: Boolean, + focused: Boolean, modifier: Modifier = Modifier ) { when (item) { ItemInList.EnabledCaption -> { - Subheader( + Caption( modifier = modifier, - selected = selected, + focused = focused, title = R.string.long_press_menu_enabled_actions, description = R.string.long_press_menu_enabled_actions_description ) } ItemInList.HiddenCaption -> { - Subheader( + Caption( modifier = modifier, - selected = selected, + focused = focused, title = R.string.long_press_menu_hidden_actions, description = R.string.long_press_menu_hidden_actions_description ) @@ -296,7 +316,7 @@ private fun ItemInListUi( is ItemInList.Action -> { ActionOrHeaderBox( modifier = modifier, - selected = selected, + focused = focused, icon = item.type.icon, text = item.type.label, contentColor = MaterialTheme.colorScheme.onSurface @@ -306,7 +326,7 @@ private fun ItemInListUi( ItemInList.HeaderBox -> { ActionOrHeaderBox( modifier = modifier, - selected = selected, + focused = focused, icon = Icons.Default.ArtTrack, text = R.string.long_press_menu_header, contentColor = MaterialTheme.colorScheme.onSurfaceVariant, @@ -318,7 +338,7 @@ private fun ItemInListUi( ItemInList.NoneMarker -> { ActionOrHeaderBox( modifier = modifier, - selected = selected, + focused = focused, icon = Icons.Default.Close, text = R.string.none, // 0.38f is the same alpha that the Material3 library applies for disabled buttons @@ -329,9 +349,11 @@ private fun ItemInListUi( is ItemInList.DragMarker -> { ActionOrHeaderBox( modifier = modifier, - selected = selected, + focused = focused, icon = Icons.Default.DragHandle, text = R.string.detail_drag_description, + // this should be just barely visible, we could even decide to hide it completely + // at some point, since it doesn't provide much of a useful hint contentColor = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.1f) ) } @@ -361,7 +383,7 @@ private fun QuickActionButtonPreview( Surface { ItemInListUi( item = itemInList, - selected = itemInList.stableUniqueKey() % 2 == 0, + focused = itemInList.stableUniqueKey() % 2 == 0, modifier = Modifier.width(MinButtonWidth * (itemInList.columnSpan ?: 4)) ) } diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorState.kt b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorState.kt index c1b9abbc0..498cef5fb 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorState.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuEditorState.kt @@ -33,12 +33,18 @@ import kotlinx.coroutines.launch private const val TAG = "LongPressMenuEditorStat" /** - * This class is very tied to [LongPressMenuEditor] and interacts with the UI layer through + * Holds a list of items (from a fixed set of items, see [ItemInList]) to show in a `LazyGrid`, and + * allows performing drag operations on this list, both via touch and via DPAD (e.g. Android TVs). + * Loads the list state (composed of whether the header is enabled and of the action arrangement) + * from settings upon initialization, and only persists changes back to settings when [onDispose] is + * called. + * + * This class is very tied to [LongPressMenuEditorPage] and interacts with the UI layer through * [gridState]. Therefore it's not a view model but rather a state holder class, see * https://developer.android.com/topic/architecture/ui-layer/stateholders#ui-logic. * - * See the javadoc of [LongPressMenuEditor] to understand which behaviors you should test for when - * changing this class. + * See the javadoc of [LongPressMenuEditorPage] to understand which behaviors you should test for + * when changing this class. */ @Stable class LongPressMenuEditorState( @@ -53,14 +59,54 @@ class LongPressMenuEditorState( return@run buildItemsInList(isHeaderEnabled, actionArrangement).toMutableStateList() } - // variables for handling drag, focus, and autoscrolling when finger is at top/bottom - var activeDragItem by mutableStateOf(null) - var activeDragPosition by mutableStateOf(IntOffset.Zero) - var activeDragSize by mutableStateOf(IntSize.Zero) - var currentlyFocusedItem by mutableIntStateOf(-1) - var autoScrollJob by mutableStateOf(null) - var autoScrollSpeed by mutableFloatStateOf(0f) + // variables for handling drag, DPAD focus, and autoscrolling when finger is at top/bottom + /** If not null, the [ItemInList] that the user picked up and is dragging around. */ + var activeDragItem by mutableStateOf(null) + private set + + /** If [activeDragItem]`!=null`, contains the user's finger position. */ + var activeDragPosition by mutableStateOf(IntOffset.Zero) + private set + + /** If [activeDragItem]`!=null`, the size it had in the list before being picked up. */ + var activeDragSize by mutableStateOf(IntSize.Zero) + private set + + /** If `>=0`, the index of the list item currently focused via DPAD (e.g. on Android TVs). */ + var currentlyFocusedItem by mutableIntStateOf(-1) + private set + + /** + * It is `!=null` only when the user is dragging something via touch, and is used to scroll + * up/down if the user's finger is close to the top/bottom of the list. + */ + private var autoScrollJob by mutableStateOf(null) + + /** + * A value in range `[0, maxSpeed]`, computed with [autoScrollSpeedFromTouchPos], and used by + * [autoScrollJob] to scroll faster or slower depending on how close the finger is to the + * top/bottom of the list. + */ + private var autoScrollSpeed by mutableFloatStateOf(0f) + + /** + * Build the initial list of [ItemInList] given the [isHeaderEnabled] and [actionArrangement] + * loaded from settings. A "hidden actions" caption will separate the enabled actions (at the + * beginning of the list) from the disabled ones (at the end). + * + * @param isHeaderEnabled whether the header should come before or after the "hidden actions" + * caption in the list + * @param actionArrangement a list of **distinct** [LongPressAction.Type]s to show before the + * "hidden actions"; items must be distinct because it wouldn't make sense to enable an action + * twice, but also because the [LongPressAction.Type]`.ordinal`s are used as `LazyGrid` IDs in + * the UI (see [ItemInList.stableUniqueKey]), which requires them to be unique, so any duplicate + * items will be removed + * @return a list with [ItemInList.Action]s of all [LongPressAction.Type]s, with a header, and + * with two textual captions in between to distinguish between enabled and disabled items, for a + * total of `#(`[LongPressAction.Type]`) + 3` items (`+ 1` if a [ItemInList.NoneMarker] is also + * needed to indicate that no items are enabled or disabled) + */ private fun buildItemsInList( isHeaderEnabled: Boolean, actionArrangement: List @@ -72,6 +118,7 @@ class LongPressMenuEditorState( } yieldAll( actionArrangement + .distinct() // see in the javadoc why this is important .map { ItemInList.Action(it) } .ifEmpty { if (isHeaderEnabled) listOf() else listOf(ItemInList.NoneMarker) } ) @@ -80,6 +127,7 @@ class LongPressMenuEditorState( yield(ItemInList.HeaderBox) } yieldAll( + // these are trivially all distinct, so no need for distinct() here LongPressAction.Type.entries .filter { !actionArrangement.contains(it) } .map { ItemInList.Action(it) } @@ -88,11 +136,21 @@ class LongPressMenuEditorState( }.toList() } + /** + * Rebuilds the list state given the default action arrangement and header enabled status. Note + * that this does not save anything to settings, but only changes the list shown in the UI, as + * per the class javadoc. + */ fun resetToDefaults(context: Context) { items.clear() - items.addAll(buildItemsInList(true, getDefaultEnabledLongPressActions(context))) + items.addAll(buildItemsInList(true, getDefaultLongPressActionArrangement(context))) } + /** + * @return the [ItemInList] at the position [offset] (relative to the start of the lazy grid), + * or the closest item along the row of the grid intersecting with [offset], or `null` if no + * such item exists + */ private fun findItemForOffsetOrClosestInRow(offset: IntOffset): LazyGridItemInfo? { var closestItemInRow: LazyGridItemInfo? = null // Using manual for loop with indices instead of firstOrNull() because this method gets @@ -109,6 +167,12 @@ class LongPressMenuEditorState( return closestItemInRow } + /** + * @return a number between 0 and [maxSpeed] indicating how fast the view should auto-scroll + * up/down while dragging an item, depending on how close the finger is to the top/bottom; uses + * this piecewise linear function, where `x=`[touchPos]`.y/height`: + * `f(x) = maxSpeed * max((x-1)/borderPercent + 1, min(x/borderPercent - 1, 0))` + */ private fun autoScrollSpeedFromTouchPos( touchPos: IntOffset, maxSpeed: Float = 20f, @@ -130,47 +194,72 @@ class LongPressMenuEditorState( } /** - * Called not just for drag gestures initiated by moving the finger, but also with DPAD's Enter. + * Prepares the list state because user wants to pick up an item, by putting the selected item + * in [activeDragItem] and replacing it in the view with a [ItemInList.DragMarker]. Called not + * just for drag gestures initiated by moving the finger, but also with DPAD's Enter. + * @param pos the touch position (for touch dragging), or the focus position (for DPAD moving) + * @param rawItem the `LazyGrid` item the user selected (it's a parameter because it's + * determined differently for touch and for DPAD) + * @return `true` if the dragging could be initiated correctly, `false` otherwise (e.g. if the + * item is not supposed to be draggable) */ - private fun beginDragGesture(pos: IntOffset, rawItem: LazyGridItemInfo) { - if (activeDragItem != null) return - val item = items.getOrNull(rawItem.index) ?: return - if (item.isDraggable) { - items[rawItem.index] = ItemInList.DragMarker(item.columnSpan) - activeDragItem = item - activeDragPosition = pos - activeDragSize = rawItem.size - } + private fun beginDrag(pos: IntOffset, rawItem: LazyGridItemInfo): Boolean { + if (activeDragItem != null) return false + val item = items.getOrNull(rawItem.index) ?: return false + if (!item.isDraggable) return false + + items[rawItem.index] = ItemInList.DragMarker(item.columnSpan) + activeDragItem = item + activeDragPosition = pos + activeDragSize = rawItem.size + return true } /** - * This beginDragGesture() overload is only called when moving the finger (not on DPAD's Enter). + * Finds the item under the user's touch, and then just delegates to [beginDrag], and if that's + * successful starts [autoScrollJob]. Only called on touch input, and not on DPAD input. Will + * not do anything if [wasLongPressed] is `false`, because only long-press-then-move should be + * used for moving items, note that however the touch events will still be forwarded to + * [handleDragChangeTouch] to handle scrolling. */ - fun beginDragGesture(pos: IntOffset, wasLongPressed: Boolean) { + fun beginDragTouch(pos: IntOffset, wasLongPressed: Boolean) { if (!wasLongPressed) { // items can be dragged around only if they are long-pressed; // use the drag as scroll otherwise return } val rawItem = findItemForOffsetOrClosestInRow(pos) ?: return - beginDragGesture(pos, rawItem) - autoScrollSpeed = 0f - autoScrollJob = coroutineScope.launch { - while (isActive) { - if (autoScrollSpeed != 0f) { - gridState.scrollBy(autoScrollSpeed) + if (beginDrag(pos, rawItem)) { + // only start the job if `beginDragGesture` was successful + autoScrollSpeed = 0f + autoScrollJob?.cancel() // just in case + autoScrollJob = coroutineScope.launch { + while (isActive) { + if (autoScrollSpeed != 0f) { + gridState.scrollBy(autoScrollSpeed) + } + delay(16L) // roughly 60 FPS } - delay(16L) // roughly 60 FPS } } } /** - * Called not just for drag gestures by moving the finger, but also with DPAD's events. + * Called when the user's finger, or the DPAD focus, moves over a new item while a drag is + * active (i.e. [activeDragItem]`!=null`). Moves the [ItemInList.DragMarker] in the list to be + * at the current position of [rawItem]/[dragItem], and adds/removes [ItemInList.NoneMarker] if + * needed. + * @param dragItem the same as [activeDragItem], but `!= null` + * @param rawItem the raw `LazyGrid` state of the [ItemInList] that the user is currently + * passing over with touch or focus */ - private fun handleDragGestureChange(dragItem: ItemInList, rawItem: LazyGridItemInfo) { + private fun handleDragChange(dragItem: ItemInList, rawItem: LazyGridItemInfo) { val prevDragMarkerIndex = items.indexOfFirst { it is ItemInList.DragMarker } - .takeIf { it >= 0 } ?: return // impossible situation, DragMarker is always in the list + .takeIf { it >= 0 } + if (prevDragMarkerIndex == null) { + Log.w(TAG, "DragMarker not being in the list should be impossible") + return + } // compute where the DragMarker will go (we need to do special logic to make sure the // HeaderBox always sticks right after EnabledCaption or HiddenCaption) @@ -212,10 +301,12 @@ class LongPressMenuEditorState( } /** - * This handleDragGestureChange() overload is only called when moving the finger - * (not on DPAD's events). + * Handles touch gesture movements, and scrolls the `LazyGrid` if no item is being actively + * dragged, or otherwise delegates to [handleDragChange]. Also updates [activeDragPosition] (so + * the dragged item can be shown at that offset in the UI) and [autoScrollSpeed]. This is only + * called on touch input, and not on DPAD input. */ - fun handleDragGestureChange(pos: IntOffset, posChangeForScrolling: Offset) { + fun handleDragChangeTouch(pos: IntOffset, posChangeForScrolling: Offset) { val dragItem = activeDragItem if (dragItem == null) { // when the user clicks outside of any draggable item, or if the user did not long-press @@ -226,17 +317,22 @@ class LongPressMenuEditorState( autoScrollSpeed = autoScrollSpeedFromTouchPos(pos) activeDragPosition = pos val rawItem = findItemForOffsetOrClosestInRow(pos) ?: return - handleDragGestureChange(dragItem, rawItem) + handleDragChange(dragItem, rawItem) } /** - * Called in multiple places, e.g. when the finger stops touching, or with DPAD events. + * Concludes the touch/DPAD drag, stops the [autoScrollJob] if any, and most importantly + * "releases" the [activeDragItem] by putting it back in the list, replacing the + * [ItemInList.DragMarker]. This function is called in multiple places, e.g. when the finger + * stops touching, or with DPAD events. */ - fun completeDragGestureAndCleanUp() { + fun completeDragAndCleanUp() { autoScrollJob?.cancel() autoScrollJob = null autoScrollSpeed = 0f + // activeDragItem could be null if the user did not long-press any item but is just + // scrolling the view, see `beginDragTouch()` and `handleDragChangeTouch()` activeDragItem?.let { dragItem -> val dragMarkerIndex = items.indexOfFirst { it is ItemInList.DragMarker } if (dragMarkerIndex >= 0) { @@ -249,27 +345,43 @@ class LongPressMenuEditorState( } /** - * Handles DPAD events on Android TVs. + * Handles DPAD events on Android TVs (right, left, up, down, center). Items can be focused by + * navigating with arrows and can be selected (thus initiating a drag) with center. Once + * selected, arrow button presses will move the item around in the list, and pressing center + * will release the item at the new position. When focusing or moving an item outside of the + * screen, the `LazyGrid` will scroll to it. + * + * @param event the event to process + * @param columns the number of columns in the `LazyGrid`, needed to correctly go one line + * up/down when receiving the up/down events + * @return `true` if the event was handled, `false` if it wasn't (if this function returns + * `false`, the event is supposed to be handled by the focus mechanism of some external view, + * e.g. to give focus back to views other than the `LazyGrid`) */ fun onKeyEvent(event: KeyEvent, columns: Int): Boolean { - if (event.type != KeyEventType.KeyDown) { - if (event.type == KeyEventType.KeyUp && - event.key == Key.DirectionDown && + // generally we only care about [KeyEventType.KeyDown] events, as is common on Android TVs, + // but in the special case where the user has an external view in focus (i.e. a button in + // the toolbar) and then presses the down-arrow to enter the `LazyGrid`, we will only + // receive [KeyEventType.KeyUp] here, and we need to handle it + if (event.type != KeyEventType.KeyDown) { // KeyDown means that the button was pressed + if (event.type == KeyEventType.KeyUp && // KeyDown means that the button was released + event.key == Key.DirectionDown && // DirectionDown indicates the down-arrow button currentlyFocusedItem < 0 ) { currentlyFocusedItem = 0 } return false } - var focusedItem = currentlyFocusedItem + + var focusedItem = currentlyFocusedItem // do operations on a local variable when (event.key) { Key.DirectionUp -> { if (focusedItem < 0) { - return false + return false // already at the beginning, } else if (items[focusedItem].columnSpan == null) { - focusedItem -= 1 + focusedItem -= 1 // this item uses the whole line, just go to the previous item } else { - // go to the previous line + // go to the item in the same column on the previous line var remaining = columns while (true) { focusedItem -= 1 @@ -286,11 +398,11 @@ class LongPressMenuEditorState( Key.DirectionDown -> { if (focusedItem >= items.size - 1) { - return false + return false // already at the end } else if (focusedItem < 0 || items[focusedItem].columnSpan == null) { - focusedItem += 1 + focusedItem += 1 // this item uses the whole line, just go to the next item } else { - // go to the next line + // go to the item in the same column on the next line var remaining = columns while (true) { focusedItem += 1 @@ -307,7 +419,7 @@ class LongPressMenuEditorState( Key.DirectionLeft -> { if (focusedItem < 0) { - return false + return false // already at the beginning } else { focusedItem -= 1 } @@ -315,39 +427,41 @@ class LongPressMenuEditorState( Key.DirectionRight -> { if (focusedItem >= items.size - 1) { - return false + return false // already at the end } else { focusedItem += 1 } } + // when pressing enter/center, either start a drag or complete the current one Key.Enter, Key.NumPadEnter, Key.DirectionCenter -> if (activeDragItem == null) { val rawItem = gridState.layoutInfo.visibleItemsInfo .firstOrNull { it.index == focusedItem } ?: return false - beginDragGesture(rawItem.offset, rawItem) + beginDrag(rawItem.offset, rawItem) return true } else { - completeDragGestureAndCleanUp() + completeDragAndCleanUp() return true } - else -> return false + else -> return false // we don't need this event } currentlyFocusedItem = focusedItem if (focusedItem < 0) { - // not checking for focusedItem>=items.size because it's impossible for it + // there is no `if (focusedItem >= items.size)` because it's impossible for it // to reach that value, and that's because we assume that there is nothing // else focusable *after* this view. This way we don't need to cleanup the // drag gestures when the user reaches the end, which would be confusing as // then there would be no indication of the current cursor position at all. - completeDragGestureAndCleanUp() + completeDragAndCleanUp() return false } else if (focusedItem >= items.size) { Log.w(TAG, "Invalid focusedItem $focusedItem: >= items size ${items.size}") } + // find the item with the closest index to handle `focusedItem < 0` or `>= items.size` cases val rawItem = gridState.layoutInfo.visibleItemsInfo .minByOrNull { abs(it.index - focusedItem) } ?: return false // no item is visible at all, impossible case @@ -356,7 +470,7 @@ class LongPressMenuEditorState( // scroll to it. Note that this will cause the "drag item" to appear misplaced, // since the drag item's position is set to the position of the focused item // before scrolling. However, it's not worth overcomplicating the logic just for - // correcting the position of a drag hint on Android TVs. + // correcting the UI position of a drag hint on Android TVs. val h = rawItem.size.height if (rawItem.index != focusedItem || rawItem.offset.y <= gridState.layoutInfo.viewportStartOffset + 0.8 * h || @@ -367,21 +481,24 @@ class LongPressMenuEditorState( } } - val dragItem = activeDragItem - if (dragItem != null) { + activeDragItem?.let { dragItem -> // This will mostly bring the drag item to the right position, but will // misplace it if the view just scrolled (see above), or if the DragMarker's // position is moved past HiddenCaption by handleDragGestureChange() below. // However, it's not worth overcomplicating the logic just for correcting - // the position of a drag hint on Android TVs. + // the UI position of a drag hint on Android TVs. activeDragPosition = rawItem.offset - handleDragGestureChange(dragItem, rawItem) + handleDragChange(dragItem, rawItem) } return true } + /** + * Stops any currently active drag, and saves to settings the action arrangement and whether the + * header is enabled. + */ fun onDispose(context: Context) { - completeDragGestureAndCleanUp() + completeDragAndCleanUp() var isHeaderEnabled = false val actionArrangement = ArrayList() @@ -418,6 +535,9 @@ sealed class ItemInList( object NoneMarker : ItemInList() data class DragMarker(override val columnSpan: Int?) : ItemInList() + /** + * @return a unique key for each [ItemInList], which can be used as a key for `Lazy` containers + */ fun stableUniqueKey(): Int { return when (this) { is Action -> this.type.ordinal diff --git a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettings.kt b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettings.kt index a32606e4d..c681dea05 100644 --- a/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettings.kt +++ b/app/src/main/java/org/schabi/newpipe/ui/components/menu/LongPressMenuSettings.kt @@ -54,7 +54,12 @@ private fun getShowPlayWithKodi(context: Context): Boolean { .getBoolean(context.getString(R.string.show_play_with_kodi_key), false) } -fun getDefaultEnabledLongPressActions(context: Context): List { +/** + * Returns the default arrangement of actions in the long press menu. Includes [PlayWithKodi] only + * if the user enabled Kodi in settings. Note however that this does not prevent the user from + * adding/removing [PlayWithKodi] anyway, via the long press menu editor. + */ +fun getDefaultLongPressActionArrangement(context: Context): List { return if (getShowPlayWithKodi(context)) { // only include Kodi in the default actions if it is enabled in settings DefaultEnabledActions + listOf(PlayWithKodi) @@ -63,12 +68,16 @@ fun getDefaultEnabledLongPressActions(context: Context): List { val key = context.getString(R.string.long_press_menu_action_arrangement_key) val ids = PreferenceManager.getDefaultSharedPreferences(context) .getString(key, null) if (ids == null) { - return getDefaultEnabledLongPressActions(context) + return getDefaultLongPressActionArrangement(context) } else if (ids.isEmpty()) { return emptyList() // apparently the user has disabled all buttons } @@ -90,10 +99,14 @@ fun loadLongPressActionArrangementFromSettings(context: Context): List) { val items = actions.joinToString(separator = ",") { it.id.toString() } val key = context.getString(R.string.long_press_menu_action_arrangement_key) @@ -102,6 +115,10 @@ fun storeLongPressActionArrangementToSettings(context: Context, actions: List( +data class Either( val value: Any, - val classA: KClass, - val classB: KClass + val classA: KClass, + val classB: KClass ) { + /** + * Calls either [ifLeft] or [ifRight] by casting the [value] this [Either] was built with to + * either [A] or [B] (first tries [A], and if that fails uses [B] and asserts that the cast + * succeeds). See [Either] for a possible pitfall of this function. + */ inline fun match(ifLeft: (A) -> R, ifRight: (B) -> R): R { return classA.safeCast(value)?.let { ifLeft(it) } ?: ifRight(classB.cast(value)) } companion object { + /** + * Builds an [Either] populated with a value of the left variant type [A]. + */ inline fun left(a: A): Either = Either(a, A::class, B::class) + + /** + * Builds an [Either] populated with a value of the right variant type [B]. + */ inline fun right(b: B): Either = Either(b, A::class, B::class) } } diff --git a/app/src/main/java/org/schabi/newpipe/util/text/FadedMarqueeModifier.kt b/app/src/main/java/org/schabi/newpipe/util/text/FadedMarqueeModifier.kt index e9d78c92c..8d88cd799 100644 --- a/app/src/main/java/org/schabi/newpipe/util/text/FadedMarqueeModifier.kt +++ b/app/src/main/java/org/schabi/newpipe/util/text/FadedMarqueeModifier.kt @@ -16,21 +16,25 @@ import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.unit.Dp /** + * A Modifier to be applied to [androidx.compose.material3.Text]. If the text is too large, this + * fades out the left and right edges of the text, and makes the text scroll horizontally, so the + * user can read it all. + * * Note: the values in [basicMarquee] are hardcoded, but feel free to expose them as parameters * in case that will be needed in the future. * * Taken from sample [androidx.compose.foundation.samples.BasicMarqueeWithFadedEdgesSample]. */ fun Modifier.fadedMarquee(edgeWidth: Dp): Modifier { - fun ContentDrawScope.drawFadedEdge(leftEdge: Boolean) { + fun ContentDrawScope.drawFadedEdge(leftOrRightEdge: Boolean) { // left = true, right = false val edgeWidthPx = edgeWidth.toPx() drawRect( - topLeft = Offset(if (leftEdge) 0f else size.width - edgeWidthPx, 0f), + topLeft = Offset(if (leftOrRightEdge) 0f else size.width - edgeWidthPx, 0f), size = Size(edgeWidthPx, size.height), brush = Brush.horizontalGradient( colors = listOf(Color.Transparent, Color.Black), - startX = if (leftEdge) 0f else size.width, - endX = if (leftEdge) edgeWidthPx else size.width - edgeWidthPx + startX = if (leftOrRightEdge) 0f else size.width, + endX = if (leftOrRightEdge) edgeWidthPx else size.width - edgeWidthPx ), blendMode = BlendMode.DstIn ) @@ -40,8 +44,8 @@ fun Modifier.fadedMarquee(edgeWidth: Dp): Modifier { .graphicsLayer { compositingStrategy = CompositingStrategy.Offscreen } .drawWithContent { drawContent() - drawFadedEdge(leftEdge = true) - drawFadedEdge(leftEdge = false) + drawFadedEdge(leftOrRightEdge = true) + drawFadedEdge(leftOrRightEdge = false) } .basicMarquee( repeatDelayMillis = 2000, diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5c6a394fb..eddc51a6a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -903,6 +903,9 @@ Background\nfrom here Popup\nfrom here Play\nfrom here + Background\nshuffled + Popup\nshuffled + Play\nshuffled Enabled actions: Reorder the actions by long pressing them and then dragging them around Hidden actions: @@ -912,9 +915,6 @@ Reset to defaults Are you sure you want to reset to the default actions? Reorder and hide actions - Background\nshuffled - Popup\nshuffled - Play\nshuffled %d items in playlist Stopped loading after %1$d pages and %2$d items to avoid rate limits diff --git a/app/src/test/java/org/schabi/newpipe/util/EitherTest.kt b/app/src/test/java/org/schabi/newpipe/util/EitherTest.kt new file mode 100644 index 000000000..6be8d8361 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/util/EitherTest.kt @@ -0,0 +1,43 @@ +package org.schabi.newpipe.util + +import org.junit.Assert.assertEquals +import org.junit.Assert.fail +import org.junit.Test + +class EitherTest { + @Test + fun testMatchLeft() { + var leftCalledTimes = 0 + Either.left("A").match( + ifLeft = { e -> + assertEquals("A", e) + leftCalledTimes += 1 + }, + ifRight = { fail() } + ) + assert(leftCalledTimes == 1) + } + + @Test + fun testMatchRight() { + var rightCalledTimes = 0 + Either.right(5).match( + ifLeft = { fail() }, + ifRight = { e -> + assertEquals(5, e) + rightCalledTimes += 1 + } + ) + assert(rightCalledTimes == 1) + } + + @Test + fun testCovariance() { + // since values can only be read from an Either, you can e.g. assign Either + // to Either because String is a subclass of Object + val e1: Either = Either.left("Hello") + assertEquals("Hello", e1.value) + val e2: Either = Either.right(5) + assertEquals(5, e2.value) + } +}