Skip to content

Commit

Permalink
Bug 1887829 - Use icon from content state in tab strip when available…
Browse files Browse the repository at this point in the history
… r=android-reviewers,007

We discussed this scenario in the [[ mozilla-mobile/firefox-android#5218 (comment) | PR adding tab strip ]] and thought simply Favicon could be used, but turns out there are cases where `ContentState.icon` has the bitmap but `Favicon` doesn't return that from the cache.

This still doesn't solve addons.mozilla.org case.

Differential Revision: https://phabricator.services.mozilla.com/D205729
  • Loading branch information
rahulsainani committed Mar 26, 2024
1 parent bcccb31 commit 7255e3c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

package org.mozilla.fenix.browser.tabstrip

import android.graphics.Bitmap
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.Image
import androidx.compose.foundation.background
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Arrangement
Expand Down Expand Up @@ -37,6 +39,7 @@ import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.graphics.asImageBitmap
import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
Expand Down Expand Up @@ -297,7 +300,10 @@ private fun TabItem(

Spacer(modifier = Modifier.size(8.dp))

TabStripIcon(state.url)
TabStripIcon(
url = state.url,
icon = state.icon,
)

Spacer(modifier = Modifier.size(8.dp))

Expand Down Expand Up @@ -332,17 +338,30 @@ private fun TabItem(
}

@Composable
private fun TabStripIcon(url: String) {
private fun TabStripIcon(
url: String,
icon: Bitmap?,
) {
Box(
modifier = Modifier
.size(tabStripIconSize)
.clip(CircleShape),
contentAlignment = Alignment.Center,
) {
Favicon(
url = url,
size = tabStripIconSize,
)
if (icon != null && !icon.isRecycled) {
Image(
bitmap = icon.asImageBitmap(),
contentDescription = null,
modifier = Modifier
.size(tabStripIconSize)
.clip(CircleShape),
)
} else {
Favicon(
url = url,
size = tabStripIconSize,
)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package org.mozilla.fenix.browser.tabstrip

import android.graphics.Bitmap
import mozilla.components.browser.state.selector.getNormalOrPrivateTabs
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.BrowserState
Expand All @@ -27,13 +28,15 @@ data class TabStripState(
* @property id The id of the tab.
* @property title The title of the tab.
* @property url The url of the tab.
* @property icon The icon of the tab.
* @property isPrivate Whether or not the tab is private.
* @property isSelected Whether or not the tab is selected.
*/
data class TabStripItem(
val id: String,
val title: String,
val url: String,
val icon: Bitmap? = null,
val isPrivate: Boolean,
val isSelected: Boolean,
)
Expand All @@ -56,6 +59,7 @@ internal fun BrowserState.toTabStripState(
id = it.id,
title = it.content.title.ifBlank { it.content.url },
url = it.content.url,
icon = it.content.icon,
isPrivate = it.content.private,
isSelected = !isSelectDisabled && it.id == selectedTabId,
)
Expand Down

0 comments on commit 7255e3c

Please sign in to comment.