tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit a9a7ade4a6aeab0da1ddae873fcc41993c905835
parent 7e3adb025ff9e4396413f5ca1943b6078535b017
Author: Akhil Pindiprolu <apindiprolu@mozilla.com>
Date:   Thu, 13 Nov 2025 16:16:15 +0000

Bug 1982739 - Migrate the DropdownMenu reusable component to match the updated M3 + Acorn spec and update the preview r=007,android-reviewers,calu

Differential Revision: https://phabricator.services.mozilla.com/D266058

Diffstat:
Mmobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/DropdownMenu.kt | 295+++++++++++++++++++++++++++++++++++++++++++------------------------------------
Mmobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/MenuItem.kt | 35++++++++++++++++++++++++++++++-----
2 files changed, 190 insertions(+), 140 deletions(-)

diff --git a/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/DropdownMenu.kt b/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/DropdownMenu.kt @@ -15,29 +15,24 @@ 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.rememberScrollState import androidx.compose.foundation.selection.selectable -import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.MenuDefaults import androidx.compose.material3.Text import androidx.compose.runtime.Composable -import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.ReadOnlyComposable -import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource @@ -58,8 +53,6 @@ import androidx.compose.material3.DropdownMenuItem as MaterialDropdownMenuItem import mozilla.components.ui.icons.R as iconsR private val MenuItemHeight = 48.dp -private val ItemHorizontalSpaceBetween = 16.dp -private val defaultMenuShape = RoundedCornerShape(8.dp) /** * A dropdown menu that displays a list of [MenuItem]s. The menu can be expanded or collapsed and @@ -82,29 +75,28 @@ fun DropdownMenu( scrollState: ScrollState = rememberScrollState(), onDismissRequest: () -> Unit, ) { - MaterialTheme(shapes = MaterialTheme.shapes.copy(medium = defaultMenuShape)) { - MaterialDropdownMenu( - expanded = expanded, + MaterialDropdownMenu( + expanded = expanded, + onDismissRequest = onDismissRequest, + modifier = modifier, + offset = offset, + scrollState = scrollState, + containerColor = MaterialTheme.colorScheme.surfaceContainerLowest, + ) { + DropdownMenuContent( + menuItems = menuItems, onDismissRequest = onDismissRequest, - offset = offset, - scrollState = scrollState, - modifier = modifier.background(AcornTheme.colors.layer2), - ) { - DropdownMenuContent( - menuItems = menuItems, - onDismissRequest = onDismissRequest, - ) + ) - val density = LocalDensity.current + val density = LocalDensity.current - LaunchedEffect(Unit) { - if (expanded) { - menuItems.indexOfFirst { - it is MenuItem.CheckableItem && it.isChecked - }.takeIf { it != -1 }?.let { index -> - val scrollPosition = with(density) { MenuItemHeight.toPx() * index }.toInt() - scrollState.scrollTo(scrollPosition) - } + LaunchedEffect(Unit) { + if (expanded) { + menuItems.indexOfFirst { + it is MenuItem.CheckableItem && it.isChecked + }.takeIf { it != -1 }?.let { index -> + val scrollPosition = with(density) { MenuItemHeight.toPx() * index }.toInt() + scrollState.scrollTo(scrollPosition) } } } @@ -119,55 +111,53 @@ private fun DropdownMenuContent( menuItems.forEach { when (it) { is MenuItem.FixedItem -> { - CompositionLocalProvider(LocalLevelColor provides it.level) { - when (it) { - is MenuItem.TextItem -> FlexibleDropdownMenuItem( - onClick = { - onDismissRequest() - it.onClick() - }, - modifier = Modifier - .testTag(it.testTag), - content = { - TextMenuItemContent(item = it) - }, - ) - - is MenuItem.IconItem -> FlexibleDropdownMenuItem( - onClick = { - onDismissRequest() - it.onClick() - }, - modifier = Modifier - .testTag(it.testTag), - content = { - IconMenuItemContent(item = it) - }, - ) - - is MenuItem.CheckableItem -> FlexibleDropdownMenuItem( - modifier = Modifier - .selectable( + when (it) { + is MenuItem.TextItem -> FlexibleDropdownMenuItem( + onClick = { + onDismissRequest() + it.onClick() + }, + enabled = it.enabled, + modifier = Modifier.testTag(it.testTag), + level = it.level, + content = { TextMenuItemContent(item = it) }, + ) + + is MenuItem.IconItem -> FlexibleDropdownMenuItem( + onClick = { + onDismissRequest() + it.onClick() + }, + enabled = it.enabled, + modifier = Modifier.testTag(it.testTag), + level = it.level, + content = { IconMenuItemContent(item = it) }, + ) + + is MenuItem.CheckableItem -> FlexibleDropdownMenuItem( + onClick = { + onDismissRequest() + it.onClick() + }, + enabled = it.enabled, + modifier = Modifier + .thenConditional( + Modifier.selectable( selected = it.isChecked, role = Role.Button, onClick = { onDismissRequest() it.onClick() }, - ) - .testTag(it.testTag) - .thenConditional( - modifier = Modifier.semantics { traversalIndex = -1f }, - ) { it.isChecked }, - onClick = { - onDismissRequest() - it.onClick() - }, - content = { - CheckableMenuItemContent(item = it) - }, - ) - } + ), + ) { it.enabled } + .testTag(it.testTag) + .thenConditional( + Modifier.semantics { traversalIndex = -1f }, + ) { it.isChecked }, + level = it.level, + content = { CheckableMenuItemContent(item = it) }, + ) } } @@ -187,7 +177,10 @@ private fun DropdownMenuContent( private fun TextMenuItemContent( item: MenuItem.TextItem, ) { - MenuItemText(item.text) + MenuItemText( + text = item.text, + supportingText = item.supportingText, + ) } @Composable @@ -197,14 +190,16 @@ private fun CheckableMenuItemContent( if (item.isChecked) { Icon( painter = painterResource(iconsR.drawable.mozac_ic_checkmark_24), - tint = AcornTheme.levelColors.iconPrimary, contentDescription = null, ) } else { Spacer(modifier = Modifier.size(24.dp)) } - MenuItemText(item.text) + MenuItemText( + text = item.text, + supportingText = item.supportingText, + ) } @Composable @@ -213,11 +208,13 @@ private fun IconMenuItemContent( ) { Icon( painter = painterResource(item.drawableRes), - tint = AcornTheme.levelColors.iconPrimary, contentDescription = null, ) - MenuItemText(item.text) + MenuItemText( + text = item.text, + supportingText = item.supportingText, + ) } @Composable @@ -225,78 +222,52 @@ private fun FlexibleDropdownMenuItem( onClick: () -> Unit, modifier: Modifier = Modifier, enabled: Boolean = true, - contentPadding: PaddingValues = MenuDefaults.DropdownMenuItemContentPadding, + contentPadding: PaddingValues = PaddingValues(all = AcornTheme.layout.space.static150), interactionSource: MutableInteractionSource? = null, + level: Level = Level.Default, content: @Composable () -> Unit, ) { MaterialDropdownMenuItem( onClick = onClick, modifier = modifier - .height(MenuItemHeight) .semantics(mergeDescendants = true) {}, enabled = enabled, contentPadding = contentPadding, interactionSource = interactionSource, text = { Row( - horizontalArrangement = Arrangement.spacedBy(ItemHorizontalSpaceBetween), + horizontalArrangement = Arrangement.spacedBy(AcornTheme.layout.space.static150), verticalAlignment = Alignment.CenterVertically, ) { content() } }, + colors = when (level) { + Level.Default -> MenuDefaults.itemColors() + Level.Critical -> MenuDefaults.itemColors(textColor = MaterialTheme.colorScheme.error) + }, ) } @Composable -private fun MenuItemText(text: Text) { - Text( - text = text.value, - color = AcornTheme.levelColors.textPrimary, - style = AcornTheme.typography.subtitle1, - ) -} - -/** - * A subset of the color palette that is used to style a UI element based on their level. - * - * @property textPrimary The primary text color. - * @property iconPrimary The primary icon color. - */ -private data class LevelColors( - val textPrimary: Color, - val iconPrimary: Color, -) - -/** - * CompositionLocal that provides the current level of importance. - */ -private val LocalLevelColor = compositionLocalOf { Level.Default } - -/** - * Returns the [LevelColors] based on the current level of importance. - */ -private val AcornTheme.levelColors: LevelColors - @Composable - @ReadOnlyComposable - get() { - val current = LocalLevelColor.current - return when (current) { - Level.Default -> { - LevelColors( - textPrimary = colors.textPrimary, - iconPrimary = colors.iconPrimary, - ) - } +private fun MenuItemText(text: Text, supportingText: Text? = null) { + Column( + verticalArrangement = Arrangement.spacedBy(2.dp), + ) { + Text( + text = text.value, + style = AcornTheme.typography.body1, + ) - Level.Critical -> { - LevelColors( - textPrimary = colors.textCritical, - iconPrimary = colors.iconCritical, - ) - } + supportingText?.let { + Text( + text = it.value, + style = AcornTheme.typography.body1, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) } } +} private data class MenuPreviewParameter( val itemType: ItemType, @@ -321,6 +292,7 @@ private val menuPreviewParameters by lazy { MenuItem.TextItem( text = Text.String("Text Item 1"), onClick = {}, + supportingText = Text.String("Supporting text"), ), ), ), @@ -331,11 +303,13 @@ private val menuPreviewParameters by lazy { text = Text.String("Checkable Item 1"), isChecked = true, onClick = {}, + supportingText = Text.String("Supporting text"), ), MenuItem.CheckableItem( text = Text.String("Checkable Item 2"), isChecked = false, onClick = {}, + supportingText = Text.String("Supporting text"), ), ), ), @@ -352,6 +326,7 @@ private val menuPreviewParameters by lazy { text = Text.String("Have a cookie!"), drawableRes = iconsR.drawable.mozac_ic_cookies_24, onClick = {}, + supportingText = Text.String("Supporting text"), ), MenuItem.Divider, MenuItem.IconItem( @@ -371,23 +346,25 @@ private fun DropdownMenuPreview() { AcornTheme { Column( modifier = Modifier - .background(color = AcornTheme.colors.layer1) + .verticalScroll(rememberScrollState()) + .background(color = MaterialTheme.colorScheme.background) .fillMaxSize() .padding(AcornTheme.layout.space.dynamic400), - verticalArrangement = Arrangement.spacedBy(AcornTheme.layout.space.dynamic400), + verticalArrangement = Arrangement.spacedBy(AcornTheme.layout.space.dynamic200), ) { Text( text = "Click buttons to expand dropdown menu", style = AcornTheme.typography.body1, - color = AcornTheme.colors.textPrimary, + color = MaterialTheme.colorScheme.onSurface, ) Text( text = """ The menu items along with checkable state should be hoisted in feature logic and simply passed to the DropdownMenu composable. The mapping is done here in the composable as an example, try to do that outside the composables. + Note: the menu does not show consistently when ran in an interactive preview. For best results, deploy the preview to a device. """.trimIndent(), style = AcornTheme.typography.caption, - color = AcornTheme.colors.textPrimary, + color = MaterialTheme.colorScheme.onSurface, ) menuPreviewParameters.forEach { @@ -409,16 +386,15 @@ private fun DropdownMenuPreview() { } } - Spacer(modifier = Modifier.size(AcornTheme.layout.space.dynamic400)) - Text( text = "Dropdown menu items", style = AcornTheme.typography.body1, - color = AcornTheme.colors.textPrimary, + color = MaterialTheme.colorScheme.onSurface, ) Column( - modifier = Modifier.background(color = AcornTheme.colors.layer2), + modifier = Modifier.background(color = MaterialTheme.colorScheme.surfaceContainerLowest), + ) { val menuItems: List<MenuItem> by remember { mutableStateOf(menuPreviewParameters.map { it.menuItems.first() }) @@ -427,16 +403,65 @@ private fun DropdownMenuPreview() { DropdownMenuContent(menuItems) { } } - Spacer(modifier = Modifier.size(AcornTheme.layout.space.dynamic400)) + Text( + text = "Dropdown menu items with dividers", + style = AcornTheme.typography.body1, + color = MaterialTheme.colorScheme.onSurface, + ) + + Column( + modifier = Modifier.background(color = MaterialTheme.colorScheme.surfaceContainerLowest), + + ) { + val menuItems: List<MenuItem> = remember { + val dividerList = mutableListOf<MenuItem>() + menuPreviewParameters.forEach { + dividerList.add(it.menuItems.first()) + dividerList.add(MenuItem.Divider) + } + dividerList + } + + DropdownMenuContent(menuItems) { } + } + + Text( + text = "Disabled menu items", + style = AcornTheme.typography.body1, + color = MaterialTheme.colorScheme.onSurface, + ) + + Column( + modifier = Modifier.background(color = MaterialTheme.colorScheme.surfaceContainerLowest), + ) { + val disabledMenuItems: List<MenuItem> = remember { + menuPreviewParameters.map { it.menuItems.first() }.map { item -> + when (item) { + is MenuItem.TextItem -> + item.copy(enabled = false) + is MenuItem.IconItem -> + item.copy(enabled = false) + is MenuItem.CheckableItem -> + item.copy(enabled = false) + is MenuItem.CustomMenuItem -> + item + is MenuItem.Divider -> + item + } + } + } + + DropdownMenuContent(disabledMenuItems) { } + } Text( text = "Checkable menu item usage", style = AcornTheme.typography.body1, - color = AcornTheme.colors.textPrimary, + color = MaterialTheme.colorScheme.onSurface, ) Column( - modifier = Modifier.background(color = AcornTheme.colors.layer2), + modifier = Modifier.background(color = MaterialTheme.colorScheme.surfaceContainerLowest), ) { var isChecked by remember { mutableStateOf(true) } diff --git a/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/MenuItem.kt b/mobile/android/android-components/components/compose/base/src/main/java/mozilla/components/compose/base/menu/MenuItem.kt @@ -23,13 +23,18 @@ sealed interface MenuItem { * @property text The text to be displayed in the menu item. * @property level The level of the menu item. This is used to determine the style of the menu. * @property testTag Tag used to identify the item in automated tests. + * @property supportingText The supporting text to be displayed below [text]. + * @property enabled Sets the enabled status for the item. By default, it is always set to true. * @property onClick The action to be performed when the menu item is clicked. + * */ sealed class FixedItem( open val text: Text, open val level: Level, open val testTag: String, + open val supportingText: Text? = null, open val onClick: () -> Unit, + open val enabled: Boolean = true, ) : MenuItem { /** @@ -54,19 +59,27 @@ sealed interface MenuItem { * @property text [Text] to be displayed in the menu item. * @property level The level of the menu item. This is used to determine the style of the menu. * @property testTag Tag used to identify the item in automated tests. + * @property supportingText The supporting text to be displayed below [text]. + * @property enabled Sets the enabled status for the item. By default, it is always set to true. * @property onClick The action to be performed when the menu item is clicked. + * */ data class TextItem( override val text: Text, override val level: Level = Level.Default, override val testTag: String = "", + override val supportingText: Text? = null, + override val enabled: Boolean = true, override val onClick: () -> Unit, - ) : FixedItem( + + ) : FixedItem( text = text, level = level, testTag = testTag, + supportingText = supportingText, + enabled = enabled, onClick = onClick, - ) + ) /** * [CheckableItem] is a [FixedItem] that represents a menu item with text and a check icon. @@ -75,6 +88,8 @@ sealed interface MenuItem { * @property isChecked The state of the checkable item. * @property level The level of the menu item. This is used to determine the style of the menu. * @property testTag Tag used to identify the item in automated tests. + * @property supportingText The supporting text to be displayed below [text]. + * @property enabled Sets the enabled status for the item. By default, it is always set to true. * @property onClick The action to be performed when the menu item is clicked. */ data class CheckableItem( @@ -82,11 +97,15 @@ sealed interface MenuItem { val isChecked: Boolean, override val level: Level = Level.Default, override val testTag: String = "", + override val supportingText: Text? = null, + override val enabled: Boolean = true, override val onClick: () -> Unit, - ) : FixedItem( + ) : FixedItem( text = text, level = level, testTag = testTag, + supportingText = supportingText, + enabled = enabled, onClick = onClick, ) @@ -97,6 +116,8 @@ sealed interface MenuItem { * @property drawableRes The drawable resource to be displayed in the menu item. * @property level The level of the menu item. This is used to determine the style of the menu. * @property testTag Tag used to identify the item in automated tests. + * @property supportingText The supporting text to be displayed below [text]. + * @property enabled Sets the enabled status for the item. By default, it is always set to true. * @property onClick The action to be performed when the menu item is clicked. */ data class IconItem( @@ -104,13 +125,17 @@ sealed interface MenuItem { @param:DrawableRes val drawableRes: Int, override val level: Level = Level.Default, override val testTag: String = "", + override val supportingText: Text? = null, + override val enabled: Boolean = true, override val onClick: () -> Unit, - ) : FixedItem( + ) : FixedItem( text = text, level = level, testTag = testTag, + supportingText = supportingText, + enabled = enabled, onClick = onClick, - ) + ) /** * [CustomMenuItem] can be used to render a custom content as a menu item. This should be used