From 86d043462889babcb68aee2a24ef26bfeae3915a Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 26 Jul 2026 22:07:08 +0200 Subject: [PATCH 1/2] fix(video): keep the "open in browser" button alive in a short error box --- .../composable/PlaybackErrorOverlayFitTest.kt | 147 ++++++++++++++++++ .../composable/RenderPlaybackError.kt | 131 +++++++++++----- 2 files changed, 236 insertions(+), 42 deletions(-) create mode 100644 amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt 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..01bb97a102 --- /dev/null +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/service/playback/composable/PlaybackErrorOverlayFitTest.kt @@ -0,0 +1,147 @@ +/* + * 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.runtime.mutableStateOf +import androidx.compose.ui.Modifier +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.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 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 starves when the content is 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 browserButtonLabel = + InstrumentationRegistry + .getInstrumentation() + .targetContext + .getString(R.string.error_video_open_in_browser) + + /** A FilledTonalButton's natural height; anything much under this is a squeezed button. */ + private val naturalButtonHeight = 40.dp + + private fun renderInBox( + width: Dp, + height: Dp, + ) { + rule.setContent { + 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", + ) + } + } + rule.waitForIdle() + } + + private fun assertButtonUsable(context: String) { + val button = rule.onNodeWithText(browserButtonLabel) + button.assertIsDisplayed() + + val bounds = button.getUnclippedBoundsInRoot() + val height = bounds.bottom - bounds.top + assertTrue( + "$context: browser button collapsed to $height (natural is $naturalButtonHeight)", + height >= naturalButtonHeight - 2.dp, + ) + } + + @Test + fun buttonSurvivesTheFeedsSixteenByNineBox() { + renderInBox(width = 322.dp, height = 181.dp) + assertButtonUsable("feed 16:9") + } + + @Test + fun buttonSurvivesTheThreadsSixteenByNineBox() { + renderInBox(width = 385.dp, height = 217.dp) + assertButtonUsable("thread 16:9") + } + + @Test + fun buttonSurvivesAnUnusuallyShortBox() { + // A 3:1 banner-ish stream, or a narrow quote card: far less height than 16:9 gives. + renderInBox(width = 322.dp, height = 110.dp) + assertButtonUsable("short box") + } + + @Test + fun descriptionIsDroppedRatherThanSlicedInHalf() { + // A weighted Text given 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) + + rule + .onNodeWithText( + InstrumentationRegistry.getInstrumentation().targetContext.getString( + R.string.error_video_playback_failed_description, + "ERROR_CODE_PARSING_MANIFEST_MALFORMED", + ), + ).assertDoesNotExist() + } + + @Test + fun titleStillShowsWhenRoomIsTight() { + renderInBox(width = 322.dp, height = 181.dp) + rule.onNodeWithText(InstrumentationRegistry.getInstrumentation().targetContext.getString(R.string.error_video_playback_failed)).assertIsDisplayed() + } +} 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..e79f49068d 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 @@ -39,6 +40,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color 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 +70,100 @@ 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, - ) + // A Column measures its children against the height left over by the ones before them, so + // the button — last in the stack — is what collapses when the overlay is taller than its + // box. That is not hypothetical: the media box is 16:9 whenever the stream reports no + // dimensions, and in the feed a note is inset under the 55dp author column, leaving 322dp + // x 181dp on a normal phone. The full layout wants ~220dp, so the button was rendering + // 0.38dp tall — in the tree, and invisible on screen. + // + // Two things keep it alive. The description takes a weight, so it is the element that + // yields when space runs short rather than the button. And the icon, which is decorative + // and costs 60dp with its spacer, is dropped when even one line of description plus the + // button would not otherwise fit. + val hasRoomForIcon = maxHeight >= MIN_HEIGHT_FOR_ICON + 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), + 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, ) - Spacer(Modifier.size(8.dp)) - Text(stringRes(R.string.error_video_open_in_browser)) + + 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. + */ +private val MIN_HEIGHT_FOR_ICON = 190.dp + +/** + * Below this the 16dp padding is itself competing with the button for space — 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 buys the button its full height back. + */ +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 From 0b1c4894a0fad818ac076cbd656ff06dde1e6235 Mon Sep 17 00:00:00 2001 From: davotoula Date: Mon, 27 Jul 2026 16:14:02 +0200 Subject: [PATCH 2/2] Code review: - scale the icon threshold so it can't slice the title - make the button's survival structural, not threshold-tuned --- .../composable/PlaybackErrorOverlayFitTest.kt | 209 +++++++++++++----- .../composable/RenderPlaybackError.kt | 87 ++++---- 2 files changed, 204 insertions(+), 92 deletions(-) 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 index 01bb97a102..dcd1867d03 100644 --- 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 @@ -23,12 +23,17 @@ 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 @@ -36,6 +41,7 @@ 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 @@ -47,7 +53,7 @@ import org.junit.runner.RunWith * 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 starves when the content is taller than the box. A note in the feed is + * 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 @@ -58,81 +64,70 @@ import org.junit.runner.RunWith class PlaybackErrorOverlayFitTest { @get:Rule val rule = createComposeRule() - private val browserButtonLabel = - InstrumentationRegistry - .getInstrumentation() - .targetContext - .getString(R.string.error_video_open_in_browser) - - /** A FilledTonalButton's natural height; anything much under this is a squeezed button. */ - private val naturalButtonHeight = 40.dp + private val targetContext = InstrumentationRegistry.getInstrumentation().targetContext private fun renderInBox( width: Dp, height: Dp, + fontScale: Float = 1f, ) { rule.setContent { - 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, + 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", - ) + ), + videoUri = "https://streamstr.net/x/hls/live.m3u8", + ) + } } } - rule.waitForIdle() } - private fun assertButtonUsable(context: String) { - val button = rule.onNodeWithText(browserButtonLabel) - button.assertIsDisplayed() - - val bounds = button.getUnclippedBoundsInRoot() - val height = bounds.bottom - bounds.top - assertTrue( - "$context: browser button collapsed to $height (natural is $naturalButtonHeight)", - height >= naturalButtonHeight - 2.dp, - ) + private fun assertButtonUsable() { + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_open_in_browser)) + .assertIsDisplayed() + .assertHeightIsAtLeast(ButtonDefaults.MinHeight) } @Test - fun buttonSurvivesTheFeedsSixteenByNineBox() { + fun buttonAndTitleSurviveTheFeedsSixteenByNineBox() { renderInBox(width = 322.dp, height = 181.dp) - assertButtonUsable("feed 16:9") + assertButtonUsable() + rule + .onNodeWithText(stringRes(targetContext, R.string.error_video_playback_failed)) + .assertIsDisplayed() } @Test fun buttonSurvivesTheThreadsSixteenByNineBox() { renderInBox(width = 385.dp, height = 217.dp) - assertButtonUsable("thread 16:9") + assertButtonUsable() } @Test - fun buttonSurvivesAnUnusuallyShortBox() { - // A 3:1 banner-ish stream, or a narrow quote card: far less height than 16:9 gives. + 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("short box") - } - - @Test - fun descriptionIsDroppedRatherThanSlicedInHalf() { - // A weighted Text given 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( - InstrumentationRegistry.getInstrumentation().targetContext.getString( + stringRes( + targetContext, R.string.error_video_playback_failed_description, "ERROR_CODE_PARSING_MANIFEST_MALFORMED", ), @@ -140,8 +135,116 @@ class PlaybackErrorOverlayFitTest { } @Test - fun titleStillShowsWhenRoomIsTight() { - renderInBox(width = 322.dp, height = 181.dp) - rule.onNodeWithText(InstrumentationRegistry.getInstrumentation().targetContext.getString(R.string.error_video_playback_failed)).assertIsDisplayed() + 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 e79f49068d..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 @@ -38,6 +38,7 @@ 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 @@ -76,18 +77,14 @@ fun RenderPlaybackError( .fillMaxSize() .background(Color.Black.copy(alpha = 0.75f)), ) { - // A Column measures its children against the height left over by the ones before them, so - // the button — last in the stack — is what collapses when the overlay is taller than its - // box. That is not hypothetical: the media box is 16:9 whenever the stream reports no - // dimensions, and in the feed a note is inset under the 55dp author column, leaving 322dp - // x 181dp on a normal phone. The full layout wants ~220dp, so the button was rendering - // 0.38dp tall — in the tree, and invisible on screen. - // - // Two things keep it alive. The description takes a weight, so it is the element that - // yields when space runs short rather than the button. And the icon, which is decorative - // and costs 60dp with its spacer, is dropped when even one line of description plus the - // button would not otherwise fit. - val hasRoomForIcon = maxHeight >= MIN_HEIGHT_FOR_ICON + // 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 @@ -96,35 +93,40 @@ fun RenderPlaybackError( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, ) { - if (hasRoomForIcon) { - Icon( - symbol = MaterialSymbols.VideocamOff, - contentDescription = null, - modifier = Modifier.size(48.dp), - tint = Color.White, - ) + 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)) + Spacer(Modifier.height(12.dp)) + } Text( - text = stringRes(R.string.error_video_playback_failed_description, errorCodeName), - color = Color.White.copy(alpha = 0.85f), - style = MaterialTheme.typography.bodySmall, + text = stringRes(R.string.error_video_playback_failed), + color = Color.White, + style = MaterialTheme.typography.titleSmall, textAlign = TextAlign.Center, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.weight(1f, fill = false), ) + + 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)) @@ -151,13 +153,20 @@ fun RenderPlaybackError( * 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 competing with the button for space — 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 buys the button its full height back. + * 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