laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-10-06 03:38:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: repair the viewer chrome defects the audit turned up

Browse Source 
Five fixes, all in the chrome the two viewers now share:

The PDF page swallowed its own tap. `zoomable` consumes the gesture before the
full-screen box underneath sees it, which is why the image path hangs its
toggle off `onTap` rather than a parent `clickable` -- so the page does too.
Without it the chrome auto-hid after two seconds and no tap could bring it
back, stranding the reader with no way out but the system gesture.

The auto-hide timer now races the controls going away instead of sleeping
through it: hiding and re-showing the chrome inside the two-second window used
to leave the original timer running, so it wiped controls the user had just
tapped back up. It also waits for the media to arrive (`armed`), because a PDF
that took longer than the delay to fetch rendered its first page with the
chrome already gone and nothing left to re-arm.

The save button ran on `rememberCoroutineScope` while living inside the
`AnimatedVisibility` that the auto-hide collapses two seconds later -- so the
chrome fading out cancelled the download it had just started, leaving no file
and no error. It now uses the view model's scope and the application context,
matching the download row in `ShareMediaAction`.

The page counter no longer slides sideways when the buttons fade: it sits in
its own centred row, anchored to the screen rather than to the space the
asymmetric button groups leave behind.

The back button also survives the loading and unreadable-PDF states, which had
inherited hidden system bars from the immersive effect without keeping a way
back out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014PQscXLTMHXHwYyKh4xcKC
This commit is contained in:
Claude
2026-08-25 15:19:03 +00:00
parent 9dc209cdfe
commit 1c25bcb8d3
2 changed files with 99 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/FullScreenViewerChrome.kt
+34 -13
View File
@@ -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<Boolean> {
fun rememberViewerControlsVisibility(
holdOpen: Boolean,
armed: Boolean = true,
): MutableState<Boolean> {
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<Boolean> {
@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 ->
amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/pdf/PdfViewerDialog.kt
+65 -51
View File
@@ -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)
},