diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt new file mode 100644 index 0000000000..dcd1867d03 --- /dev/null +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt @@ -0,0 +1,250 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.width +import androidx.compose.material3.ButtonDefaults +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.mutableStateOf +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.test.assertHeightIsAtLeast +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.getUnclippedBoundsInRoot +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import androidx.media3.common.PlaybackException +import androidx.media3.common.Player +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.ui.stringRes +import io.mockk.mockk +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * The "open in browser" fallback is the only thing this overlay offers the user, so it has to + * survive whatever box the media layout hands it. + * + * [Column] measures children in declaration order against the remaining height, so the button — + * being last — is what starved when the content was taller than the box. A note in the feed is + * inset under the 55dp author column (screenWidth - 89dp), and with no imeta `dim` the media box + * is 16:9, which on a 411dp phone is 322dp wide and only 181dp tall. That was enough to squeeze + * the button down to 0.38dp — present in the tree, invisible and untappable on screen — while the + * same note opened in the thread (full bleed, screenWidth - 26dp, so a 217dp box) rendered it at + * 35.8dp. + */ +@RunWith(AndroidJUnit4::class) +class PlaybackErrorOverlayFitTest { + @get:Rule val rule = createComposeRule() + + private val targetContext = InstrumentationRegistry.getInstrumentation().targetContext + + private fun renderInBox( + width: Dp, + height: Dp, + fontScale: Float = 1f, + ) { + rule.setContent { + val density = LocalDensity.current.density + CompositionLocalProvider(LocalDensity provides Density(density, fontScale)) { + Box(Modifier.width(width).height(height)) { + RenderPlaybackError( + controllerState = + MediaControllerState( + controller = mockk(relaxed = true), + playbackError = + mutableStateOf( + PlaybackException( + "Malformed HLS manifest", + null, + PlaybackException.ERROR_CODE_PARSING_MANIFEST_MALFORMED, + ), + ), + ), + videoUri = "https://streamstr.net/x/hls/live.m3u8", + ) + } + } + } + } + + private fun assertButtonUsable() { + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_open_in_browser)) + .assertIsDisplayed() + .assertHeightIsAtLeast(ButtonDefaults.MinHeight) + } + + @Test + fun buttonAndTitleSurviveTheFeedsSixteenByNineBox() { + renderInBox(width = 322.dp, height = 181.dp) + assertButtonUsable() + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_playback_failed)) + .assertIsDisplayed() + } + + @Test + fun buttonSurvivesTheThreadsSixteenByNineBox() { + renderInBox(width = 385.dp, height = 217.dp) + assertButtonUsable() + } + + @Test + fun shortBoxKeepsTheButtonAndDropsTheDescription() { + // A 3:1 banner-ish stream, or a narrow quote card: far less height than 16:9 gives. A + // weighted Text handed less than one line's height draws it clipped through the middle, + // which looks broken — under that much pressure it should not be emitted at all. + renderInBox(width = 322.dp, height = 110.dp) + assertButtonUsable() + rule + .onNodeWithText( + stringRes( + targetContext, + R.string.error_video_playback_failed_description, + "ERROR_CODE_PARSING_MANIFEST_MALFORMED", + ), + ).assertDoesNotExist() + } + + @Test + fun buttonSurvivesLargeFontScale() { + // At fontScale 2 the text wants far more height than the dp thresholds were tuned for. + // The button must still get its intrinsic height, because it is measured before the + // weighted text block — the guarantee is structural, not numeric. + renderInBox(width = 322.dp, height = 195.dp, fontScale = 2f) + assertButtonUsable() + } + + @Test + fun shrinkingTheBoxNeverShrinksTheButton() { + // ButtonDefaults.MinHeight alone does not pin the guarantee: a button squeezed from its + // intrinsic 53dp down to 40dp at fontScale 2 still clears that floor. What actually has to + // hold is that the box height cannot influence the button's height at all, because the + // button is measured before the weighted text block that absorbs the shortfall. Measure + // the same button roomy and then at its tightest, and require the two to agree. + val boxHeight = mutableStateOf(400.dp) + + rule.setContent { + val density = LocalDensity.current.density + CompositionLocalProvider(LocalDensity provides Density(density, 2f)) { + Box(Modifier.width(322.dp).height(boxHeight.value)) { + RenderPlaybackError( + controllerState = + MediaControllerState( + controller = mockk(relaxed = true), + playbackError = + mutableStateOf( + PlaybackException( + "Malformed HLS manifest", + null, + PlaybackException.ERROR_CODE_PARSING_MANIFEST_MALFORMED, + ), + ), + ), + videoUri = "https://streamstr.net/x/hls/live.m3u8", + ) + } + } + } + + val roomy = buttonHeight() + + // 190dp is the worst case for the old threshold-only layout: just enough to keep the icon, + // not enough to pay for it, so the shortfall landed on the button. + rule.runOnUiThread { boxHeight.value = 190.dp } + rule.waitForIdle() + + val tight = buttonHeight() + assertTrue( + "button shrank from $roomy to $tight when the box did", + tight >= roomy - 1.dp, + ) + } + + @Test + fun theIconNeverCostsTheTitleItsHeight() { + // The icon is non-weighted and declared first, so inside the weighted text block it is + // measured before the title. With a fixed 190dp gate, a box of 190dp to 199dp at fontScale 2 + // was just tall enough to keep the icon and not tall enough to pay for it, so the title + // rendered sliced. Decoration must yield before words do. + val boxHeight = mutableStateOf(400.dp) + + rule.setContent { + val density = LocalDensity.current.density + CompositionLocalProvider(LocalDensity provides Density(density, 2f)) { + Box(Modifier.width(322.dp).height(boxHeight.value)) { + RenderPlaybackError( + controllerState = + MediaControllerState( + controller = mockk(relaxed = true), + playbackError = + mutableStateOf( + PlaybackException( + "Malformed HLS manifest", + null, + PlaybackException.ERROR_CODE_PARSING_MANIFEST_MALFORMED, + ), + ), + ), + videoUri = "https://streamstr.net/x/hls/live.m3u8", + ) + } + } + } + + val roomy = titleHeight() + + rule.runOnUiThread { boxHeight.value = 195.dp } + rule.waitForIdle() + + val tight = titleHeight() + assertTrue( + "title sliced from $roomy to $tight to make room for the decorative icon", + tight >= roomy - 1.dp, + ) + } + + private fun titleHeight(): Dp { + val bounds = + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_playback_failed)) + .getUnclippedBoundsInRoot() + return bounds.bottom - bounds.top + } + + private fun buttonHeight(): Dp { + val bounds = + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_open_in_browser)) + .getUnclippedBoundsInRoot() + return bounds.bottom - bounds.top + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderPlaybackError.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderPlaybackError.kt index 8b8742c4e6..b0322cf50c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderPlaybackError.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderPlaybackError.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.service.playback.composable import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize @@ -37,8 +38,10 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.commons.icons.symbols.Icon @@ -68,55 +71,108 @@ fun RenderPlaybackError( val uriHandler = LocalUriHandler.current val errorCodeName = remember(current) { current.errorCodeName } - Column( + BoxWithConstraints( modifier = modifier .fillMaxSize() - .background(Color.Black.copy(alpha = 0.75f)) - .padding(16.dp), - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally, + .background(Color.Black.copy(alpha = 0.75f)), ) { - Icon( - symbol = MaterialSymbols.VideocamOff, - contentDescription = null, - modifier = Modifier.size(48.dp), - tint = Color.White, - ) + // The button is the overlay's whole point, so it must never be the child that starves — + // and a Column starves whoever is measured last. In the feed a 16:9 media box can be as + // small as 322dp x 181dp, which used to squeeze the button (last in the stack) to 0.38dp. + // Putting everything above it in one weighted block makes the Column measure the button + // first and hand only the leftover to the text, so the squeeze structurally cannot land + // on the button at any box height, font scale, or locale. The thresholds below only + // decide how the text block degrades while it yields. + val hasRoomForIcon = maxHeight >= MIN_HEIGHT_FOR_ICON * LocalDensity.current.fontScale + val hasRoomForDescription = maxHeight >= MIN_HEIGHT_FOR_DESCRIPTION + val padding = if (maxHeight >= MIN_HEIGHT_FOR_FULL_PADDING) 16.dp else 8.dp - Spacer(Modifier.height(12.dp)) - - Text( - text = stringRes(R.string.error_video_playback_failed), - color = Color.White, - style = MaterialTheme.typography.titleSmall, - textAlign = TextAlign.Center, - ) - - Spacer(Modifier.height(4.dp)) - - Text( - text = stringRes(R.string.error_video_playback_failed_description, errorCodeName), - color = Color.White.copy(alpha = 0.85f), - style = MaterialTheme.typography.bodySmall, - textAlign = TextAlign.Center, - ) - - Spacer(Modifier.height(16.dp)) - - FilledTonalButton( - onClick = { - runCatching { uriHandler.openUri(videoUri) } - .onFailure { Log.w("RenderPlaybackError", "openUri failed for $videoUri", it) } - }, + Column( + modifier = Modifier.fillMaxSize().padding(padding), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally, ) { - Icon( - symbol = MaterialSymbols.OpenInBrowser, - contentDescription = null, - modifier = Modifier.size(18.dp), - ) - Spacer(Modifier.size(8.dp)) - Text(stringRes(R.string.error_video_open_in_browser)) + Column( + modifier = Modifier.weight(1f, fill = false), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + if (hasRoomForIcon) { + Icon( + symbol = MaterialSymbols.VideocamOff, + contentDescription = null, + modifier = Modifier.size(48.dp), + tint = Color.White, + ) + + Spacer(Modifier.height(12.dp)) + } + + Text( + text = stringRes(R.string.error_video_playback_failed), + color = Color.White, + style = MaterialTheme.typography.titleSmall, + textAlign = TextAlign.Center, + ) + + if (hasRoomForDescription) { + Spacer(Modifier.height(4.dp)) + + Text( + text = stringRes(R.string.error_video_playback_failed_description, errorCodeName), + color = Color.White.copy(alpha = 0.85f), + style = MaterialTheme.typography.bodySmall, + textAlign = TextAlign.Center, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f, fill = false), + ) + } + } + + Spacer(Modifier.height(16.dp)) + + FilledTonalButton( + onClick = { + runCatching { uriHandler.openUri(videoUri) } + .onFailure { Log.w("RenderPlaybackError", "openUri failed for $videoUri", it) } + }, + ) { + Icon( + symbol = MaterialSymbols.OpenInBrowser, + contentDescription = null, + modifier = Modifier.size(18.dp), + ) + Spacer(Modifier.size(8.dp)) + Text(stringRes(R.string.error_video_open_in_browser)) + } } } } + +/** + * Smallest overlay height that still fits the 48dp icon and its 12dp spacer on top of the title, + * one line of description, the 16dp gap and the 40dp button, inside 16dp of padding. Below this + * the icon is the first thing to go, because it is the only part carrying no information. + * + * Multiplied by the current `fontScale` at the call site, because everything it is budgeting + * against is text. The icon is non-weighted and declared first, so within the weighted block it is + * measured before the title — left unscaled, a box of 190dp to 199dp at fontScale 2 kept the icon + * and sliced the title to pay for it. Scaling over-corrects slightly, since the icon and paddings + * are fixed dp, but this threshold is cosmetic-only now: erring towards dropping decoration is the + * harmless direction. + */ +private val MIN_HEIGHT_FOR_ICON = 190.dp + +/** + * Below this the 16dp padding is itself crowding out the text — a very wide, short video (a + * panorama, or anything past about 2.5:1) leaves barely more height than the title and button + * need. Halving the padding there gives the title room to draw whole. + */ +private val MIN_HEIGHT_FOR_FULL_PADDING = 150.dp + +/** + * Below this the description cannot fit even one full line, and a weighted Text handed less than a + * line's height draws it sliced in half rather than dropping it. Hide it instead: the title still + * says what went wrong and the button still offers the way out. + */ +private val MIN_HEIGHT_FOR_DESCRIPTION = 120.dp