Code review:

Dead videoGroup parameter in inner RenderTopButtons overload. Confirmed via git show origin/main — the branch introduced it, and it's only passed through but never read inside the body (the outer overload uses
  videoGroup separately for VideoQualityPopup). Removed from signature and both call sites (preview + outer wrapper).

RenderTopButtons runs for every visible video tile, so recomputing the
top-bar / overflow action lists on every recomposition added avoidable
per-frame filter+map allocations. Memoize on the inputs that actually
change availability.

Also drop the outer Box around AnimatedOverflowMenuButton — the inner
OverflowMenuButton already provides its own sized anchor Box, so the
wrapper added no layout value.
This commit is contained in:
davotoula
2026-04-23 17:02:12 +02:00
parent d0b2af752e
commit 78e21a0274
2 changed files with 25 additions and 18 deletions
@@ -81,7 +81,6 @@ fun RenderTopButtonsPreview() {
Box(Modifier.background(BitcoinOrange)) {
RenderTopButtons(
mediaData = MediaItemData("http://test.mp4"),
videoGroup = null,
hasMultipleQualities = false,
qualityButton = {},
controllerVisible = remember { mutableStateOf(true) },
@@ -141,7 +140,6 @@ fun RenderTopButtons(
RenderTopButtons(
mediaData = mediaData,
videoGroup = videoGroup,
hasMultipleQualities = hasMultipleQualities,
qualityButton = {
VideoQualityButton(
@@ -186,7 +184,6 @@ fun RenderTopButtons(
@Composable
fun RenderTopButtons(
mediaData: MediaItemData,
videoGroup: Tracks.Group?,
hasMultipleQualities: Boolean,
qualityButton: @Composable () -> Unit,
controllerVisible: MutableState<Boolean>,
@@ -217,8 +214,19 @@ fun RenderTopButtons(
VideoPlayerAction.PictureInPicture -> pipSupported
}
val topBarActions = buttonItems.filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) }.map { it.action }
val overflowActions = buttonItems.filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) }.map { it.action }
val canFullscreen = onZoomClick != null
val topBarActions =
remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) {
buttonItems
.filter { it.location == VideoButtonLocation.TopBar && isAvailable(it.action) }
.map { it.action }
}
val overflowActions =
remember(buttonItems, canFullscreen, hasMultipleQualities, isLive, pipSupported) {
buttonItems
.filter { it.location == VideoButtonLocation.OverflowMenu && isAvailable(it.action) }
.map { it.action }
}
Row(modifier) {
topBarActions.forEach { action ->
@@ -274,7 +282,6 @@ fun RenderTopButtons(
}
if (overflowActions.isNotEmpty()) {
Box {
AnimatedOverflowMenuButton(
controllerVisible = controllerVisible,
actions = overflowActions,
@@ -287,7 +294,6 @@ fun RenderTopButtons(
onPipClick = onPictureInPictureClick,
)
}
}
if (shareDialogVisible.value) {
ShareMediaAction(
@@ -216,6 +216,7 @@ private fun hasNonDefaultFilters(
@OptIn(ExperimentalMaterial3Api::class)
@Composable
@Suppress("AssignedValueIsNeverRead")
private fun SearchFilterRow(searchBarViewModel: SearchBarViewModel) {
val currentScope by searchBarViewModel.scope.collectAsStateWithLifecycle()
val currentSource by searchBarViewModel.source.collectAsStateWithLifecycle()