commit c2446dba1bbbd73f79803707e2d8a9acd8f15cf8
parent ecc2f60f296d13c9e17b9936257bcab828c5715f
Author: Noah Bond <nbond@mozilla.com>
Date: Thu, 18 Dec 2025 03:04:12 +0000
Bug 2003682 - Optimize the thumbnail size calculation of the tab grid thumbnails r=android-reviewers,jdelorenzo
During the redesign, we needed some more robust thumbnail calculations in order to support the updated design specs. This led to the individual tab grid items being wrapped with `BoxWithConstraints`, so each item could get its precise measurement to calculate the thumbnail size. However, this led to a performance degradation when combining this into a `LazyVerticalGrid`, as these measurements were occurring frequently and redundantly, as all grid items were of equal size.
To solve for this, the parent `LazyVerticalGrid` is now wrapped by the `BoxWithConstraints`, and the measurement happens once for _all_ tab items.
Differential Revision: https://phabricator.services.mozilla.com/D276571
Diffstat:
2 files changed, 116 insertions(+), 94 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/ui/tabitems/TabGridItem.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/ui/tabitems/TabGridItem.kt
@@ -11,7 +11,6 @@ import androidx.compose.foundation.combinedClickable
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.isSystemInDarkTheme
import androidx.compose.foundation.layout.Box
-import androidx.compose.foundation.layout.BoxWithConstraints
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
@@ -40,7 +39,6 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.graphics.asImageBitmap
import androidx.compose.ui.platform.LocalContext
-import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
@@ -68,11 +66,14 @@ import org.mozilla.fenix.tabstray.TabsTrayTestTag
import org.mozilla.fenix.tabstray.ext.toDisplayTitle
import org.mozilla.fenix.tabstray.ui.sharedTabTransition
import org.mozilla.fenix.theme.FirefoxTheme
-import kotlin.math.max
import mozilla.components.ui.icons.R as iconsR
+/**
+ * The padding around the thumbnail inside a tab grid item.
+ */
+val GridItemThumbnailPadding = 4.dp
+
private val TabContentCardShape = RoundedCornerShape(16.dp)
-private val ThumbnailPadding = 4.dp
private val ThumbnailShape = RoundedCornerShape(
topStart = 4.dp,
topEnd = 4.dp,
@@ -87,6 +88,7 @@ private val TabHeaderFaviconSize = 12.dp
* long clicks, multiple selection, and media controls.
*
* @param tab The given tab to be render as view a grid item.
+ * @param thumbnailSizePx The size of the tab's thumbnail in pixels.
* @param isSelected Indicates if the item should be render as selected.
* @param multiSelectionEnabled Indicates if the item should be render with multi selection options,
* enabled.
@@ -101,6 +103,7 @@ private val TabHeaderFaviconSize = 12.dp
@Composable
fun TabGridItem(
tab: TabSessionState,
+ thumbnailSizePx: Int = 50,
isSelected: Boolean = false,
multiSelectionEnabled: Boolean = false,
multiSelectionSelected: Boolean = false,
@@ -110,31 +113,24 @@ fun TabGridItem(
onClick: (tab: TabSessionState) -> Unit,
onLongClick: ((tab: TabSessionState) -> Unit)? = null,
) {
- BoxWithConstraints {
- val density = LocalDensity.current
- val thumbnailWidth = this.constraints.minWidth - with(density) { 2 * ThumbnailPadding.roundToPx() }
- val thumbnailHeight = (thumbnailWidth / gridItemAspectRatio).toInt()
- val thumbnailSize = max(thumbnailWidth, thumbnailHeight)
-
- SwipeToDismissBox2(
- state = swipeState,
- backgroundContent = {},
- onItemDismiss = {
- onCloseClick(tab)
- },
- ) {
- TabContent(
- tab = tab,
- thumbnailSize = thumbnailSize,
- isSelected = isSelected,
- multiSelectionEnabled = multiSelectionEnabled,
- multiSelectionSelected = multiSelectionSelected,
- shouldClickListen = shouldClickListen,
- onCloseClick = onCloseClick,
- onClick = onClick,
- onLongClick = onLongClick,
- )
- }
+ SwipeToDismissBox2(
+ state = swipeState,
+ backgroundContent = {},
+ onItemDismiss = {
+ onCloseClick(tab)
+ },
+ ) {
+ TabContent(
+ tab = tab,
+ thumbnailSize = thumbnailSizePx,
+ isSelected = isSelected,
+ multiSelectionEnabled = multiSelectionEnabled,
+ multiSelectionSelected = multiSelectionSelected,
+ shouldClickListen = shouldClickListen,
+ onCloseClick = onCloseClick,
+ onClick = onClick,
+ onLongClick = onLongClick,
+ )
}
}
@@ -206,7 +202,7 @@ private fun TabContent(
.wrapContentHeight(),
verticalAlignment = Alignment.CenterVertically,
) {
- Spacer(modifier = Modifier.width(FirefoxTheme.layout.space.static50 + ThumbnailPadding))
+ Spacer(modifier = Modifier.width(FirefoxTheme.layout.space.static50 + GridItemThumbnailPadding))
val icon = tab.content.icon
if (icon != null) {
@@ -298,7 +294,7 @@ private fun TabContent(
Card(
modifier = Modifier
.aspectRatio(gridItemAspectRatio)
- .padding(horizontal = ThumbnailPadding),
+ .padding(horizontal = GridItemThumbnailPadding),
shape = ThumbnailShape,
) {
Thumbnail(
@@ -307,7 +303,7 @@ private fun TabContent(
)
}
- Spacer(modifier = Modifier.height(ThumbnailPadding))
+ Spacer(modifier = Modifier.height(GridItemThumbnailPadding))
}
}
}
@@ -317,7 +313,7 @@ private fun TabContent(
* The width to height ratio of the tab grid item. In landscape mode, the width to height ratio is
* 2:1 and in portrait mode, the width to height ratio is 4:5.
*/
-private val gridItemAspectRatio: Float
+val gridItemAspectRatio: Float
@Composable
@ReadOnlyComposable
get() = if (LocalContext.current.isLandscape()) {
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/ui/tabpage/TabLayout.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/tabstray/ui/tabpage/TabLayout.kt
@@ -10,6 +10,8 @@ import androidx.compose.animation.rememberSplineBasedDecay
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.BoxWithConstraints
+import androidx.compose.foundation.layout.BoxWithConstraintsScope
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
@@ -47,6 +49,7 @@ import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.tooling.preview.PreviewLightDark
import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.unit.dp
import mozilla.components.browser.state.state.ContentState
@@ -61,9 +64,12 @@ import org.mozilla.fenix.tabstray.browser.compose.createGridReorderState
import org.mozilla.fenix.tabstray.browser.compose.createListReorderState
import org.mozilla.fenix.tabstray.browser.compose.detectGridPressAndDragGestures
import org.mozilla.fenix.tabstray.browser.compose.detectListPressAndDrag
+import org.mozilla.fenix.tabstray.ui.tabitems.GridItemThumbnailPadding
import org.mozilla.fenix.tabstray.ui.tabitems.TabGridItem
import org.mozilla.fenix.tabstray.ui.tabitems.TabListItem
+import org.mozilla.fenix.tabstray.ui.tabitems.gridItemAspectRatio
import org.mozilla.fenix.theme.FirefoxTheme
+import kotlin.math.max
// Key for the span item at the bottom of the tray, used to make the item not reorderable.
const val SPAN_ITEM_KEY = "span"
@@ -212,77 +218,97 @@ private fun TabGrid(
}
}
- LazyVerticalGrid(
- columns = GridCells.Fixed(count = numberOfGridColumns),
- modifier = modifier
- .fillMaxSize()
- .detectGridPressAndDragGestures(
- gridState = state,
- reorderState = reorderState,
- shouldLongPressToDrag = shouldLongPress,
+ BoxWithConstraints {
+ LazyVerticalGrid(
+ columns = GridCells.Fixed(count = numberOfGridColumns),
+ modifier = modifier
+ .fillMaxSize()
+ .detectGridPressAndDragGestures(
+ gridState = state,
+ reorderState = reorderState,
+ shouldLongPressToDrag = shouldLongPress,
+ ),
+ state = state,
+ contentPadding = PaddingValues(
+ horizontal = if (LocalContext.current.isLandscape()) {
+ 52.dp
+ } else {
+ FirefoxTheme.layout.space.static200
+ },
+ vertical = 24.dp,
),
- state = state,
- contentPadding = PaddingValues(
- horizontal = if (LocalContext.current.isLandscape()) {
- 52.dp
- } else {
- FirefoxTheme.layout.space.static200
- },
- vertical = 24.dp,
- ),
- verticalArrangement = Arrangement.spacedBy(space = FirefoxTheme.layout.space.static200),
- horizontalArrangement = Arrangement.spacedBy(space = FirefoxTheme.layout.space.static200),
- ) {
- header?.let {
- item(key = HEADER_ITEM_KEY, span = { GridItemSpan(maxLineSpan) }) {
- header()
+ verticalArrangement = Arrangement.spacedBy(space = FirefoxTheme.layout.space.static200),
+ horizontalArrangement = Arrangement.spacedBy(space = horizontalGridPadding),
+ ) {
+ header?.let {
+ item(key = HEADER_ITEM_KEY, span = { GridItemSpan(maxLineSpan) }) {
+ header()
+ }
}
- }
- itemsIndexed(
- items = tabs,
- key = { _, tab -> tab.id },
- ) { index, tab ->
- val decayAnimationSpec: DecayAnimationSpec<Float> = rememberSplineBasedDecay()
- val density = LocalDensity.current
- val isRtl = LocalLayoutDirection.current == LayoutDirection.Rtl
- val swipeState = remember(isInMultiSelectMode, !state.isScrollInProgress) {
- SwipeToDismissState2(
- density = density,
- enabled = !isInMultiSelectMode && !state.isScrollInProgress,
- decayAnimationSpec = decayAnimationSpec,
- isRtl = isRtl,
- )
- }
- val swipingActive by remember(swipeState.swipingActive) {
- mutableStateOf(swipeState.swipingActive)
- }
+ itemsIndexed(
+ items = tabs,
+ key = { _, tab -> tab.id },
+ ) { index, tab ->
+ val decayAnimationSpec: DecayAnimationSpec<Float> = rememberSplineBasedDecay()
+ val density = LocalDensity.current
+ val isRtl = LocalLayoutDirection.current == LayoutDirection.Rtl
+ val swipeState = remember(isInMultiSelectMode, !state.isScrollInProgress) {
+ SwipeToDismissState2(
+ density = density,
+ enabled = !isInMultiSelectMode && !state.isScrollInProgress,
+ decayAnimationSpec = decayAnimationSpec,
+ isRtl = isRtl,
+ )
+ }
+ val swipingActive by remember(swipeState.swipingActive) {
+ mutableStateOf(swipeState.swipingActive)
+ }
- DragItemContainer(
- state = reorderState,
- position = index + if (header != null) 1 else 0,
- key = tab.id,
- swipingActive = swipingActive,
- ) {
- TabGridItem(
- tab = tab,
- isSelected = tab.id == selectedTabId,
- multiSelectionEnabled = isInMultiSelectMode,
- multiSelectionSelected = selectionMode.selectedTabs.any { it.id == tab.id },
- shouldClickListen = reorderState.draggingItemKey != tab.id,
- swipeState = swipeState,
- onCloseClick = onTabClose,
- onClick = onTabClick,
- )
+ DragItemContainer(
+ state = reorderState,
+ position = index + if (header != null) 1 else 0,
+ key = tab.id,
+ swipingActive = swipingActive,
+ ) {
+ TabGridItem(
+ tab = tab,
+ thumbnailSizePx = thumbnailSizePx,
+ isSelected = tab.id == selectedTabId,
+ multiSelectionEnabled = isInMultiSelectMode,
+ multiSelectionSelected = selectionMode.selectedTabs.any { it.id == tab.id },
+ shouldClickListen = reorderState.draggingItemKey != tab.id,
+ swipeState = swipeState,
+ onCloseClick = onTabClose,
+ onClick = onTabClick,
+ )
+ }
}
- }
- item(key = SPAN_ITEM_KEY, span = { GridItemSpan(maxLineSpan) }) {
- Spacer(modifier = Modifier.height(tabListBottomPadding))
+ item(key = SPAN_ITEM_KEY, span = { GridItemSpan(maxLineSpan) }) {
+ Spacer(modifier = Modifier.height(tabListBottomPadding))
+ }
}
}
}
+private val horizontalGridPadding: Dp
+ @ReadOnlyComposable
+ @Composable
+ get() = FirefoxTheme.layout.space.static200
+
+private val BoxWithConstraintsScope.thumbnailSizePx: Int
+ @ReadOnlyComposable
+ @Composable
+ get() {
+ val density = LocalDensity.current
+ val totalSpacing = horizontalGridPadding * (numberOfGridColumns - 1) +
+ GridItemThumbnailPadding * numberOfGridColumns * 2
+ val thumbnailWidth = constraints.maxWidth - with(density) { totalSpacing.roundToPx() }
+ val thumbnailHeight = (thumbnailWidth / gridItemAspectRatio).toInt()
+ return max(thumbnailWidth, thumbnailHeight)
+ }
+
@Suppress("LongParameterList", "LongMethod", "CognitiveComplexMethod")
@Composable
private fun TabList(
