Bug 1929611 - Update tabstrip item min width and close button logic r=android-reviewers,007

Differential Revision: https://phabricator.services.mozilla.com/D228199
This commit is contained in:
rahulsainani 2024-11-11 09:57:51 +00:00
parent 73af5482be
commit c99108c911
3 changed files with 101 additions and 27 deletions

View File

@ -81,7 +81,7 @@ import org.mozilla.fenix.theme.FirefoxColors
import org.mozilla.fenix.theme.FirefoxTheme
import org.mozilla.fenix.theme.Theme
private val minTabStripItemWidth = 160.dp
private val minTabStripItemWidth = 130.dp
private val maxTabStripItemWidth = 280.dp
private val tabItemHeight = 40.dp
private val tabStripIconSize = 24.dp
@ -413,26 +413,30 @@ private fun TabItem(
}
}
IconButton(
onClick = { onCloseTabClick(state.id, state.isPrivate) },
modifier = if (state.isSelected) {
Modifier.semantics {}
} else {
Modifier.clearAndSetSemantics {}
},
) {
Icon(
painter = painterResource(R.drawable.mozac_ic_cross_20),
tint = if (state.isSelected) {
FirefoxTheme.colors.iconPrimary
if (state.isCloseButtonVisible) {
IconButton(
onClick = { onCloseTabClick(state.id, state.isPrivate) },
modifier = if (state.isSelected) {
Modifier.semantics {}
} else {
FirefoxTheme.colors.iconSecondary
Modifier.clearAndSetSemantics {}
},
contentDescription = stringResource(
id = R.string.close_tab_title,
state.title,
),
)
) {
Icon(
painter = painterResource(R.drawable.mozac_ic_cross_20),
tint = if (state.isSelected) {
FirefoxTheme.colors.iconPrimary
} else {
FirefoxTheme.colors.iconSecondary
},
contentDescription = stringResource(
id = R.string.close_tab_title,
state.title,
),
)
}
} else {
Spacer(modifier = Modifier.size(8.dp))
}
}
}

View File

@ -10,6 +10,8 @@ import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.TabSessionState
private const val MAX_TABS_WITH_CLOSE_BUTTON_VISIBLE = 7
/**
* The ui state of the tabs strip.
*
@ -45,6 +47,7 @@ data class TabStripState(
* @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.
* @property isCloseButtonVisible Whether or not the close button is visible.
*/
data class TabStripItem(
val id: String,
@ -53,6 +56,7 @@ data class TabStripItem(
val icon: Bitmap? = null,
val isPrivate: Boolean,
val isSelected: Boolean,
val isCloseButtonVisible: Boolean = true,
)
/**
@ -87,6 +91,7 @@ internal fun BrowserState.toTabStripState(
it.toTabStripItem(
isSelectDisabled = isSelectDisabled,
selectedTabId = selectedTabId,
showCloseButtonOnUnselectedTabs = tabs.size <= MAX_TABS_WITH_CLOSE_BUTTON_VISIBLE,
)
},
isPrivateMode = isPrivateMode,
@ -144,11 +149,16 @@ private fun mapToMenuItems(
private fun TabSessionState.toTabStripItem(
isSelectDisabled: Boolean,
selectedTabId: String?,
) = TabStripItem(
id = id,
title = content.title.ifBlank { content.url },
url = content.url,
icon = content.icon,
isPrivate = content.private,
isSelected = !isSelectDisabled && id == selectedTabId,
)
showCloseButtonOnUnselectedTabs: Boolean,
): TabStripItem {
val isSelected = !isSelectDisabled && id == selectedTabId
return TabStripItem(
id = id,
title = content.title.ifBlank { content.url },
url = content.url,
icon = content.icon,
isPrivate = content.private,
isSelected = isSelected,
isCloseButtonVisible = showCloseButtonOnUnselectedTabs || isSelected,
)
}

View File

@ -490,6 +490,66 @@ class TabStripStateTest {
assertEquals(Pair(false, 2), closTabParams)
}
@Test
fun `WHEN more than 7 tabs are present THEN close button should only be visible for the selected tab`() {
val tab = createTab(
url = "https://example.com",
title = "Example",
private = false,
id = "1",
)
val tabs = List(8) {
tab.copy(id = it.toString())
}
val browserState = BrowserState(
tabs = tabs,
selectedTabId = "1",
)
val actual =
browserState.toTabStripState(
isSelectDisabled = false,
isPossiblyPrivateMode = false,
addTab = {},
toggleBrowsingMode = {},
closeTab = { _, _ -> },
)
val tabStripItem = TabStripItem(
id = "0",
title = "Example",
url = "https://example.com",
isSelected = false,
isPrivate = false,
isCloseButtonVisible = false,
)
val expected = TabStripState(
tabs = listOf(
tabStripItem,
TabStripItem(
id = "1",
title = "Example",
url = "https://example.com",
isSelected = true,
isPrivate = false,
isCloseButtonVisible = true,
),
tabStripItem.copy(id = "2"),
tabStripItem.copy(id = "3"),
tabStripItem.copy(id = "4"),
tabStripItem.copy(id = "5"),
tabStripItem.copy(id = "6"),
tabStripItem.copy(id = "7"),
),
isPrivateMode = false,
tabCounterMenuItems = allMenuItems,
)
expected isSameAs actual
}
/**
* Asserts that the [TabStripState] is the same as the [other] [TabStripState] by comparing
* their properties as assertEquals does. This ignores the lambda references in the