diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/FullScreenViewerChrome.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/FullScreenViewerChrome.kt index 0b594cb4ef..8434fe2c8c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/FullScreenViewerChrome.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/FullScreenViewerChrome.kt @@ -24,6 +24,7 @@ import android.Manifest import android.os.Build import android.view.Window import android.widget.Toast +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Arrangement.spacedBy import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.PaddingValues @@ -46,13 +47,14 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalView import androidx.core.view.WindowInsetsCompat import androidx.core.view.WindowInsetsControllerCompat +import androidx.lifecycle.viewModelScope import com.google.accompanist.permissions.ExperimentalPermissionsApi import com.google.accompanist.permissions.isGranted import com.google.accompanist.permissions.rememberPermissionState @@ -67,8 +69,9 @@ import com.vitorpamplona.amethyst.ui.theme.Size15dp import com.vitorpamplona.amethyst.ui.theme.Size20Modifier import com.vitorpamplona.amethyst.ui.theme.Size5dp import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull // Chrome shared by the full-screen media viewers -- the zoomable image/video dialog and the PDF // viewer. Both are opened the same way (tap a media card in a feed), so they immerse, auto-hide, @@ -99,18 +102,31 @@ fun ImmersiveSystemBarsEffect(window: Window?) { * [CONTROLS_AUTO_HIDE_DELAY_MS], and the caller flips the returned state on tap. * * [holdOpen] freezes the timer while something anchored to the controls -- the share sheet, say -- - * is up, and re-arms it once that closes. A tap that brings the controls back deliberately gets no - * timer: the user asked for them, so they stay until tapped away. + * is up, and re-arms it once that closes. [armed] withholds the countdown until there is something + * to look at, so a viewer that spends three seconds fetching its media doesn't reveal the first + * frame with the controls already gone. + * + * A tap that brings the controls back deliberately gets no timer: the user asked for them, so they + * stay until tapped away. That is why the countdown races the controls going away rather than just + * sleeping -- a timer left over from an earlier show would otherwise wipe controls the user tapped + * back up in the meantime. */ @Composable -fun rememberViewerControlsVisibility(holdOpen: Boolean): MutableState { +fun rememberViewerControlsVisibility( + holdOpen: Boolean, + armed: Boolean = true, +): MutableState { val visible = remember { mutableStateOf(true) } - LaunchedEffect(holdOpen) { - if (!holdOpen && visible.value) { - delay(CONTROLS_AUTO_HIDE_DELAY_MS) - visible.value = false - } + LaunchedEffect(armed, holdOpen) { + if (!armed || holdOpen) return@LaunchedEffect + + val hiddenFirst = + withTimeoutOrNull(CONTROLS_AUTO_HIDE_DELAY_MS) { + snapshotFlow { visible.value }.first { !it } + } + + if (hiddenFirst == null) visible.value = false } return visible @@ -134,6 +150,7 @@ fun rememberViewerControlsVisibility(holdOpen: Boolean): MutableState { @OptIn(ExperimentalLayoutApi::class) fun ViewerControlsRow( modifier: Modifier = Modifier, + horizontalArrangement: Arrangement.Horizontal = spacedBy(Size10dp), content: @Composable RowScope.() -> Unit, ) { Row( @@ -144,7 +161,7 @@ fun ViewerControlsRow( ).padding(horizontal = Size15dp, vertical = Size10dp) .fillMaxWidth() .heightIn(min = ButtonDefaults.MinHeight), - horizontalArrangement = spacedBy(Size10dp), + horizontalArrangement = horizontalArrangement, verticalAlignment = Alignment.CenterVertically, content = content, ) @@ -202,8 +219,12 @@ fun ViewerSaveToGalleryButton( content: BaseMediaContent, accountViewModel: AccountViewModel, ) { - val localContext = LocalContext.current - val scope = rememberCoroutineScope() + // The application context and the view model's scope, never the composition's: this button + // lives inside the AnimatedVisibility that the auto-hide collapses two seconds after the tap + // that started the download, and a rememberCoroutineScope job would be cancelled with it -- + // killing the save with no file and no error. Matches the download row in ShareMediaAction. + val localContext = LocalContext.current.applicationContext + val scope = accountViewModel.viewModelScope val writeStoragePermissionState = rememberPermissionState(Manifest.permission.WRITE_EXTERNAL_STORAGE) { isGranted -> diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/pdf/PdfViewerDialog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/pdf/PdfViewerDialog.kt index 2527f39163..8f45d619fe 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/pdf/PdfViewerDialog.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/pdf/PdfViewerDialog.kt @@ -29,6 +29,7 @@ import androidx.compose.animation.fadeOut import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Arrangement.spacedBy import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Row @@ -225,46 +226,44 @@ private fun PdfViewerContent( } val sharePopupExpanded = remember { mutableStateOf(false) } - val controlsVisible = rememberViewerControlsVisibility(holdOpen = sharePopupExpanded.value) - val handle = handleState - if (handle == null) { - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator(color = Color.White) - } - } else if (handle.pageCount == 0) { - Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { - Text( - text = "Unable to open PDF", - color = Color.White, - ) - } - } else { - val pagerState = rememberPagerState { handle.pageCount } - val pageCache = remember(handle) { PageBitmapCache(PAGE_CACHE_SIZE) } - // The page counter is wayfinding rather than a control, so it outlives the buttons for a - // moment after every page turn -- a reader who tapped the chrome away still sees where a - // swipe landed. - var pageJustChanged by remember { mutableStateOf(false) } - LaunchedEffect(pagerState.currentPage) { - pageJustChanged = true - delay(PAGE_INDICATOR_FLASH_MS) - pageJustChanged = false - } + // A PDF that takes longer than the auto-hide delay to fetch would otherwise reveal its first + // page with the chrome already gone, and nothing left to re-arm the timer. + val controlsVisible = + rememberViewerControlsVisibility( + holdOpen = sharePopupExpanded.value, + armed = handle != null, + ) - Box( - modifier = - Modifier - .fillMaxSize() - .clickable( - onClick = { - if (!sharePopupExpanded.value) { - controlsVisible.value = !controlsVisible.value - } - }, - ), - ) { + val pagerState = rememberPagerState { handle?.pageCount ?: 0 } + val pageCache = remember(handle) { PageBitmapCache(PAGE_CACHE_SIZE) } + + // The page counter is wayfinding rather than a control, so it outlives the buttons for a + // moment after every page turn -- a reader who tapped the chrome away still sees where a + // swipe landed. + var pageJustChanged by remember { mutableStateOf(false) } + LaunchedEffect(pagerState.currentPage) { + pageJustChanged = true + delay(PAGE_INDICATOR_FLASH_MS) + pageJustChanged = false + } + + val toggleControls = { if (!sharePopupExpanded.value) controlsVisible.value = !controlsVisible.value } + + Box(modifier = Modifier.fillMaxSize().clickable(onClick = toggleControls)) { + if (handle == null) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + CircularProgressIndicator(color = Color.White) + } + } else if (handle.pageCount == 0) { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + Text( + text = "Unable to open PDF", + color = Color.White, + ) + } + } else { HorizontalPager( state = pagerState, modifier = Modifier.fillMaxSize(), @@ -273,16 +272,39 @@ private fun PdfViewerContent( handle = handle, pageIndex = pageIndex, cache = pageCache, + // The zoomable page consumes the tap before the box underneath ever sees it, + // so the toggle has to hang off the gesture detector that owns it. + onTap = toggleControls, ) } + } - ViewerControlsRow(modifier = Modifier.align(Alignment.TopCenter)) { + // Two rows over the same strip: the buttons keep the edges, and the counter stays centred + // on the screen rather than on whatever space the buttons leave -- otherwise it slides + // sideways every time the asymmetric button groups fade out from under it. + ViewerControlsRow(modifier = Modifier.align(Alignment.TopCenter)) { + AnimatedVisibility(visible = controlsVisible.value, enter = fadeIn(), exit = fadeOut()) { + ViewerBackButton(onDismiss) + } + + Spacer(modifier = Modifier.weight(1f)) + + if (handle != null) { AnimatedVisibility(visible = controlsVisible.value, enter = fadeIn(), exit = fadeOut()) { - ViewerBackButton(onDismiss) + Row(horizontalArrangement = spacedBy(Size10dp)) { + ViewerShareButton(content, sharePopupExpanded, accountViewModel) + + ViewerSaveToGalleryButton(content, accountViewModel) + } } + } + } - Spacer(modifier = Modifier.weight(1f)) - + if (handle != null && handle.pageCount > 0) { + ViewerControlsRow( + modifier = Modifier.align(Alignment.TopCenter), + horizontalArrangement = Arrangement.Center, + ) { AnimatedVisibility( visible = controlsVisible.value || pageJustChanged, enter = fadeIn(), @@ -297,16 +319,6 @@ private fun PdfViewerContent( .padding(horizontal = Size10dp, vertical = Size5dp), ) } - - Spacer(modifier = Modifier.weight(1f)) - - AnimatedVisibility(visible = controlsVisible.value, enter = fadeIn(), exit = fadeOut()) { - Row(horizontalArrangement = spacedBy(Size10dp)) { - ViewerShareButton(content, sharePopupExpanded, accountViewModel) - - ViewerSaveToGalleryButton(content, accountViewModel) - } - } } } } @@ -318,6 +330,7 @@ private fun PdfPageView( handle: PdfDocumentHandle, pageIndex: Int, cache: PageBitmapCache, + onTap: () -> Unit, ) { val cached = cache.get(pageIndex) @@ -379,6 +392,7 @@ private fun PdfPageView( .fillMaxSize() .zoomable( zoomState = zoomState, + onTap = { onTap() }, onDoubleTap = { position -> zoomState.toggleScale(targetScale = DOUBLE_TAP_ZOOM_SCALE, position = position) },