From 5351f59940d7c4b9772f4014ff62f6fc3819f15d Mon Sep 17 00:00:00 2001 From: t-p-white Date: Tue, 26 Sep 2023 09:47:22 +0100 Subject: [PATCH] Bug 1821721 - Fixed the recently visited row width. Updated layout to match Figma designs and behaviour for parity with iOS. --- .../home/recentvisits/view/RecentlyVisited.kt | 199 ++++++++++++------ .../view/RecentlyVisitedViewHolder.kt | 6 - 2 files changed, 129 insertions(+), 76 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisited.kt b/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisited.kt index 1da20152a1..5c54e86fed 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisited.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisited.kt @@ -8,21 +8,19 @@ import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.Image import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.isSystemInDarkTheme -import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width -import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.LazyRow -import androidx.compose.foundation.lazy.itemsIndexed -import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.Card import androidx.compose.material.Text @@ -41,16 +39,16 @@ import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.testTag import androidx.compose.ui.semantics.testTagsAsResourceId import androidx.compose.ui.text.style.TextOverflow -import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import mozilla.components.support.ktx.kotlin.trimmed import org.mozilla.fenix.R import org.mozilla.fenix.compose.ContextualMenu import org.mozilla.fenix.compose.Divider -import org.mozilla.fenix.compose.EagerFlingBehavior import org.mozilla.fenix.compose.Favicon import org.mozilla.fenix.compose.MenuItem +import org.mozilla.fenix.compose.annotation.LightDarkPreview import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryGroup import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem.RecentHistoryHighlight @@ -59,6 +57,28 @@ import org.mozilla.fenix.theme.FirefoxTheme // Number of recently visited items per column. private const val VISITS_PER_COLUMN = 3 +private val itemRowHeight = 56.dp +private val horizontalArrangementSpacing = 32.dp +private val contentPadding = 16.dp +private val imageSize = 24.dp +private val imageSpacer = 16.dp + +/** + * The [Dp] width of UI elements to deduct from the screen width for a single column. + * + * Box start padding, Row start padding, Box end padding, Row end padding. + */ +private val singleColumnWidth: Dp = contentPadding * 4 + +/** + * The [Dp] width of UI elements to deduct from the screen width for multiple columns to show (peek) the + * second column icons. + * + * Box start padding, Row start padding, Spacer, Image size, Image spacer. + */ +private val multipleColumnsWidth: Dp = + contentPadding + contentPadding + horizontalArrangementSpacing + imageSize + imageSpacer + /** * A list of recently visited items. * @@ -75,51 +95,46 @@ fun RecentlyVisited( backgroundColor: Color = FirefoxTheme.colors.layer2, onRecentVisitClick: (RecentlyVisitedItem, Int) -> Unit = { _, _ -> }, ) { - Card( - modifier = Modifier.fillMaxWidth(), - shape = RoundedCornerShape(8.dp), - backgroundColor = backgroundColor, - elevation = 6.dp, - ) { - val listState = rememberLazyListState() - val flingBehavior = EagerFlingBehavior(lazyRowState = listState) + val itemsMatrix: List> = recentVisits.chunked(VISITS_PER_COLUMN) - LazyRow( - modifier = Modifier.semantics { + BoxWithConstraints( + modifier = Modifier + .fillMaxWidth() + .semantics { testTagsAsResourceId = true testTag = "recent.visits" }, - state = listState, - contentPadding = PaddingValues(16.dp), - horizontalArrangement = Arrangement.spacedBy(32.dp), - flingBehavior = flingBehavior, + ) { + val boxWithConstraintsScope = this + + val isSingleColumn = itemsMatrix.size == 1 + val widthToDeduct = if (isSingleColumn) { + singleColumnWidth + } else { + multipleColumnsWidth + } + val rowWidth = boxWithConstraintsScope.maxWidth - widthToDeduct + + LazyRow( + modifier = Modifier.fillMaxWidth(), + contentPadding = PaddingValues(horizontal = contentPadding), ) { - val itemsList = recentVisits.chunked(VISITS_PER_COLUMN) - - itemsIndexed(itemsList) { pageIndex, items -> - Column( - modifier = Modifier.fillMaxWidth(), - ) { - items.forEachIndexed { index, recentVisit -> - when (recentVisit) { - is RecentHistoryHighlight -> RecentlyVisitedHistoryHighlight( - recentVisit = recentVisit, - menuItems = menuItems, - clickableEnabled = listState.atLeastHalfVisibleItems.contains(pageIndex), - showDividerLine = index < items.size - 1, - onRecentVisitClick = { - onRecentVisitClick(it, pageIndex + 1) - }, - ) - is RecentHistoryGroup -> RecentlyVisitedHistoryGroup( - recentVisit = recentVisit, + item { + RecentlyVisitedCard(backgroundColor) { + Row(modifier = Modifier.padding(contentPadding)) { + itemsMatrix.mapIndexed { pageIndex, items -> + RecentlyVisitedColumn( + modifier = Modifier.width(rowWidth), menuItems = menuItems, - clickableEnabled = listState.atLeastHalfVisibleItems.contains(pageIndex), - showDividerLine = index < items.size - 1, - onRecentVisitClick = { - onRecentVisitClick(it, pageIndex + 1) - }, + items = items, + pageIndex = pageIndex, + onRecentVisitClick = onRecentVisitClick, ) + + val isLastColumn = pageIndex == itemsMatrix.lastIndex + if (!isLastColumn) { + Spacer(modifier = Modifier.width(horizontalArrangementSpacing)) + } } } } @@ -128,12 +143,56 @@ fun RecentlyVisited( } } +@Composable +private fun RecentlyVisitedCard(backgroundColor: Color, content: @Composable () -> Unit) { + Card( + modifier = Modifier.fillMaxWidth(), + shape = RoundedCornerShape(8.dp), + backgroundColor = backgroundColor, + elevation = 6.dp, + ) { + content() + } +} + +@Composable +private fun RecentlyVisitedColumn( + modifier: Modifier, + menuItems: List, + items: List, + pageIndex: Int, + onRecentVisitClick: (RecentlyVisitedItem, Int) -> Unit = { _, _ -> }, +) { + Column(modifier = modifier) { + items.forEachIndexed { index, recentVisit -> + when (recentVisit) { + is RecentHistoryHighlight -> RecentlyVisitedHistoryHighlight( + recentVisit = recentVisit, + menuItems = menuItems, + showDividerLine = index < items.size - 1, + onRecentVisitClick = { + onRecentVisitClick(it, pageIndex + 1) + }, + ) + + is RecentHistoryGroup -> RecentlyVisitedHistoryGroup( + recentVisit = recentVisit, + menuItems = menuItems, + showDividerLine = index < items.size - 1, + onRecentVisitClick = { + onRecentVisitClick(it, pageIndex + 1) + }, + ) + } + } + } +} + /** * A recently visited history group. * * @param recentVisit The [RecentHistoryGroup] to display. * @param menuItems List of [RecentVisitMenuItem] to display in a recent visit dropdown menu. - * @param clickableEnabled Whether click actions should be invoked or not. * @param showDividerLine Whether to show a divider line at the bottom. * @param onRecentVisitClick Invoked when the user clicks on a recent visit. */ @@ -145,7 +204,6 @@ fun RecentlyVisited( private fun RecentlyVisitedHistoryGroup( recentVisit: RecentHistoryGroup, menuItems: List, - clickableEnabled: Boolean, showDividerLine: Boolean, onRecentVisitClick: (RecentHistoryGroup) -> Unit = { _ -> }, ) { @@ -154,11 +212,11 @@ private fun RecentlyVisitedHistoryGroup( Row( modifier = Modifier .combinedClickable( - enabled = clickableEnabled, onClick = { onRecentVisitClick(recentVisit) }, onLongClick = { isMenuExpanded = true }, ) - .size(268.dp, 56.dp) + .height(itemRowHeight) + .fillMaxWidth() .semantics { testTagsAsResourceId = true testTag = "recent.visits.group" @@ -168,10 +226,10 @@ private fun RecentlyVisitedHistoryGroup( Image( painter = painterResource(R.drawable.ic_multiple_tabs), contentDescription = null, - modifier = Modifier.size(24.dp), + modifier = Modifier.size(imageSize), ) - Spacer(modifier = Modifier.width(16.dp)) + Spacer(modifier = Modifier.width(imageSpacer)) Column( modifier = Modifier.fillMaxSize(), @@ -219,7 +277,6 @@ private fun RecentlyVisitedHistoryGroup( * * @param recentVisit The [RecentHistoryHighlight] to display. * @param menuItems List of [RecentVisitMenuItem] to display in a recent visit dropdown menu. - * @param clickableEnabled Whether click actions should be invoked or not. * @param showDividerLine Whether to show a divider line at the bottom. * @param onRecentVisitClick Invoked when the user clicks on a recent visit. */ @@ -231,7 +288,6 @@ private fun RecentlyVisitedHistoryGroup( private fun RecentlyVisitedHistoryHighlight( recentVisit: RecentHistoryHighlight, menuItems: List, - clickableEnabled: Boolean, showDividerLine: Boolean, onRecentVisitClick: (RecentHistoryHighlight) -> Unit = { _ -> }, ) { @@ -240,20 +296,20 @@ private fun RecentlyVisitedHistoryHighlight( Row( modifier = Modifier .combinedClickable( - enabled = clickableEnabled, onClick = { onRecentVisitClick(recentVisit) }, onLongClick = { isMenuExpanded = true }, ) - .size(268.dp, 56.dp) + .height(itemRowHeight) + .fillMaxWidth() .semantics { testTagsAsResourceId = true testTag = "recent.visits.highlight" }, verticalAlignment = Alignment.CenterVertically, ) { - Favicon(url = recentVisit.url, size = 24.dp) + Favicon(url = recentVisit.url, size = imageSize) - Spacer(modifier = Modifier.width(16.dp)) + Spacer(modifier = Modifier.width(imageSpacer)) Box(modifier = Modifier.fillMaxSize()) { RecentlyVisitedTitle( @@ -334,21 +390,9 @@ private fun RecentlyVisitedCaption( ) } -/** - * Get the indexes in list of all items which have more than half showing. - */ -private val LazyListState.atLeastHalfVisibleItems - get() = layoutInfo - .visibleItemsInfo - .filter { - val startEdge = maxOf(0, layoutInfo.viewportStartOffset - it.offset) - val endEdge = maxOf(0, it.offset + it.size - layoutInfo.viewportEndOffset) - return@filter startEdge + endEdge < it.size / 2 - }.map { it.index } - @Composable -@Preview -private fun RecentlyVisitedPreview() { +@LightDarkPreview +private fun RecentlyVisitedMultipleColumnsPreview() { FirefoxTheme { RecentlyVisited( recentVisits = listOf( @@ -361,3 +405,18 @@ private fun RecentlyVisitedPreview() { ) } } + +@Composable +@LightDarkPreview +private fun RecentlyVisitedSingleColumnPreview() { + FirefoxTheme { + RecentlyVisited( + recentVisits = listOf( + RecentHistoryGroup(title = "running shoes"), + RecentHistoryGroup(title = "mozilla"), + RecentHistoryGroup(title = "firefox"), + ), + menuItems = emptyList(), + ) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisitedViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisitedViewHolder.kt index 01e72df5a6..29b82f8697 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisitedViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentvisits/view/RecentlyVisitedViewHolder.kt @@ -35,12 +35,6 @@ class RecentlyVisitedViewHolder( private val interactor: RecentVisitsInteractor, ) : ComposeViewHolder(composeView, viewLifecycleOwner) { - init { - val horizontalPadding = - composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) - composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) - } - @Composable override fun Content() { val recentVisits = components.appStore