From 633377e903d6ef381947f26c5799e64d19d449f6 Mon Sep 17 00:00:00 2001 From: sarah541 Date: Tue, 3 May 2022 14:32:45 -0400 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/23966 - Migrate MessageCardViewHolder to Compose --- .../perf/StartupExcessiveResourceUseTest.kt | 4 +- .../fenix/compose/CollectionsPlaceholder.kt | 2 +- .../org/mozilla/fenix/compose/MessageCard.kt | 218 ++++++++++++++++++ .../sessioncontrol/SessionControlAdapter.kt | 10 +- .../onboarding/MessageCardViewHolder.kt | 76 +++--- .../main/res/layout/nimbus_message_card.xml | 58 ----- 6 files changed, 265 insertions(+), 103 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt delete mode 100644 app/src/main/res/layout/nimbus_message_card.xml diff --git a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt index cb790c15da..ca7d2c9ade 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -25,9 +25,9 @@ private const val EXPECTED_SUPPRESSION_COUNT = 19 @Suppress("TopLevelPropertyNaming") // it's silly this would have a different naming convention b/c no const private val EXPECTED_RUNBLOCKING_RANGE = 0..1 // CI has +1 counts compared to local runs: increment these together private val EXPECTED_RECYCLER_VIEW_CONSTRAINT_LAYOUT_CHILDREN = - 4..5 // The messaging framework is not deterministic and could add a +1 to the count + 3..4 // The messaging framework is not deterministic and could add a +1 to the count private val EXPECTED_NUMBER_OF_INFLATION = - 14..15 // The messaging framework is not deterministic and could add a +1 to the count + 13..14 // The messaging framework is not deterministic and could add a +1 to the count private val failureMsgStrictMode = getErrorMessage( shortName = "StrictMode suppression", diff --git a/app/src/main/java/org/mozilla/fenix/compose/CollectionsPlaceholder.kt b/app/src/main/java/org/mozilla/fenix/compose/CollectionsPlaceholder.kt index 00fc885c59..358b28ae55 100644 --- a/app/src/main/java/org/mozilla/fenix/compose/CollectionsPlaceholder.kt +++ b/app/src/main/java/org/mozilla/fenix/compose/CollectionsPlaceholder.kt @@ -72,7 +72,7 @@ fun CollectionsPlaceholder( modifier = Modifier.size(20.dp), ) { Icon( - painter = painterResource(R.drawable.ic_close), + painter = painterResource(R.drawable.mozac_ic_close_20), contentDescription = stringResource( R.string.remove_home_collection_placeholder_content_description ), diff --git a/app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt b/app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt new file mode 100644 index 0000000000..b4dbb25716 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt @@ -0,0 +1,218 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.compose + +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material.Card +import androidx.compose.material.Icon +import androidx.compose.material.IconButton +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import androidx.compose.ui.unit.sp +import org.mozilla.experiments.nimbus.StringHolder +import org.mozilla.fenix.R +import org.mozilla.fenix.compose.button.PrimaryButton +import org.mozilla.fenix.gleanplumb.Message +import org.mozilla.fenix.nimbus.MessageData +import org.mozilla.fenix.nimbus.StyleData +import org.mozilla.fenix.theme.FirefoxTheme +import org.mozilla.fenix.theme.Theme + +/** + * Message Card. + * + * @param message [Message] that holds a representation of GleanPlum message from Nimbus. + * @param onClick Invoked when user clicks on the message card. + * @param onCloseButtonClick Invoked when user clicks on close button to remove message. + */ +@Suppress("LongMethod") +@Composable +fun MessageCard( + message: Message, + onClick: () -> Unit, + onCloseButtonClick: () -> Unit, +) { + Card( + modifier = Modifier + .padding(vertical = 16.dp) + .then( + if (message.data.buttonLabel.isNullOrBlank()) { + Modifier.clickable(onClick = onClick) + } else { + Modifier + } + ), + shape = RoundedCornerShape(8.dp), + backgroundColor = FirefoxTheme.colors.layer2, + ) { + Column( + Modifier + .padding(all = 16.dp) + .fillMaxWidth() + ) { + val title = message.data.title + if (!title.isNullOrBlank()) { + Row( + modifier = Modifier.fillMaxWidth(), + ) { + SectionHeader( + text = title, + modifier = Modifier + .weight(1f) + ) + + IconButton( + modifier = Modifier.size(20.dp), + onClick = onCloseButtonClick + ) { + Icon( + painter = painterResource(R.drawable.mozac_ic_close_20), + contentDescription = stringResource( + R.string.content_description_close_button + ), + tint = FirefoxTheme.colors.iconPrimary + ) + } + } + + PrimaryText( + text = message.data.text, + modifier = Modifier.fillMaxWidth(), + fontSize = 14.sp + ) + } else { + Row( + modifier = Modifier.fillMaxWidth(), + ) { + PrimaryText( + text = message.data.text, + modifier = Modifier.weight(1f), + fontSize = 14.sp + ) + + IconButton( + modifier = Modifier.size(20.dp), + onClick = onCloseButtonClick + ) { + Icon( + painter = painterResource(R.drawable.mozac_ic_close_20), + contentDescription = stringResource( + R.string.content_description_close_button + ), + tint = FirefoxTheme.colors.iconPrimary + ) + } + } + } + + if (!message.data.buttonLabel.isNullOrBlank()) { + Spacer(modifier = Modifier.height(16.dp)) + + PrimaryButton( + text = stringResource(R.string.preferences_set_as_default_browser), + onClick = onClick + ) + } + } + } +} + +@Composable +@Preview +private fun MessageCardPreview() { + FirefoxTheme(theme = Theme.getTheme(isPrivate = false)) { + Box(Modifier.background(FirefoxTheme.colors.layer1)) { + MessageCard( + message = Message( + id = "end-", + data = MessageData( + title = StringHolder( + R.string.bookmark_empty_title_error, + "Title" + ), + text = StringHolder( + R.string.default_browser_experiment_card_text, "description" + ) + ), + action = "action", + style = StyleData(), + triggers = listOf("trigger"), + metadata = Message.Metadata("end-") + ), + onClick = {}, + onCloseButtonClick = {} + ) + } + } +} + +@Composable +@Preview +private fun MessageCardWithoutTitlePreview() { + FirefoxTheme(theme = Theme.getTheme(isPrivate = false)) { + Box(Modifier.background(FirefoxTheme.colors.layer1)) { + MessageCard( + message = Message( + id = "end-", + data = MessageData( + text = StringHolder( + R.string.default_browser_experiment_card_text, "description" + ) + ), + action = "action", + style = StyleData(), + triggers = listOf("trigger"), + metadata = Message.Metadata("end-") + ), + onClick = {}, + onCloseButtonClick = {} + ) + } + } +} + +@Composable +@Preview +private fun MessageCardWithButtonLabelPreview() { + FirefoxTheme(theme = Theme.getTheme(isPrivate = false)) { + Box(Modifier.background(FirefoxTheme.colors.layer1)) { + MessageCard( + message = Message( + id = "end-", + data = MessageData( + buttonLabel = StringHolder(R.string.preferences_set_as_default_browser, ""), + title = StringHolder( + R.string.bookmark_empty_title_error, + "Title" + ), + text = StringHolder( + R.string.default_browser_experiment_card_text, "description" + ) + ), + action = "action", + style = StyleData(), + triggers = listOf("trigger"), + metadata = Message.Metadata("end-") + ), + onClick = {}, + onCloseButtonClick = {} + ) + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt index 0020078ff8..080fa059d3 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt @@ -293,6 +293,11 @@ class SessionControlAdapter( viewLifecycleOwner = viewLifecycleOwner, interactor = interactor, ) + MessageCardViewHolder.LAYOUT_ID -> return MessageCardViewHolder( + composeView = ComposeView(parent.context), + viewLifecycleOwner = viewLifecycleOwner, + interactor = interactor + ) } val view = LayoutInflater.from(parent.context).inflate(viewType, parent, false) @@ -318,7 +323,6 @@ class SessionControlAdapter( OnboardingToolbarPositionPickerViewHolder.LAYOUT_ID -> OnboardingToolbarPositionPickerViewHolder( view ) - MessageCardViewHolder.LAYOUT_ID -> MessageCardViewHolder(view, interactor) BottomSpacerViewHolder.LAYOUT_ID -> BottomSpacerViewHolder(view) else -> throw IllegalStateException() } @@ -328,6 +332,7 @@ class SessionControlAdapter( when (holder) { is CollectionHeaderViewHolder, is CustomizeHomeButtonViewHolder, + is MessageCardViewHolder, is NoCollectionsMessageViewHolder, is RecentlyVisitedViewHolder, is RecentVisitsHeaderViewHolder, @@ -390,9 +395,6 @@ class SessionControlAdapter( is TopSitePagerViewHolder -> { holder.bind((item as AdapterItem.TopSitePager).topSites) } - is MessageCardViewHolder -> { - holder.bind((item as AdapterItem.NimbusMessageCard).message) - } is CollectionViewHolder -> { val (collection, expanded) = item as AdapterItem.CollectionItem holder.bindSession(collection, expanded) diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/MessageCardViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/MessageCardViewHolder.kt index 8af3c3b240..ce0bfd6092 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/MessageCardViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/viewholders/onboarding/MessageCardViewHolder.kt @@ -5,53 +5,53 @@ package org.mozilla.fenix.home.sessioncontrol.viewholders.onboarding import android.view.View -import androidx.core.view.isVisible -import androidx.recyclerview.widget.RecyclerView +import androidx.compose.runtime.Composable +import androidx.compose.ui.platform.ComposeView +import androidx.lifecycle.LifecycleOwner +import mozilla.components.lib.state.ext.observeAsComposableState import org.mozilla.fenix.R -import org.mozilla.fenix.databinding.NimbusMessageCardBinding -import org.mozilla.fenix.ext.increaseTapArea -import org.mozilla.fenix.gleanplumb.Message +import org.mozilla.fenix.components.components +import org.mozilla.fenix.compose.ComposeViewHolder +import org.mozilla.fenix.compose.MessageCard import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor +/** + * View holder for the Nimbus Message Card. + * + * @property interactor [SessionControlInteractor] which will have delegated to all user + * interactions. + */ class MessageCardViewHolder( - view: View, - private val interactor: SessionControlInteractor -) : RecyclerView.ViewHolder(view) { + composeView: ComposeView, + viewLifecycleOwner: LifecycleOwner, + private val interactor: SessionControlInteractor, +) : ComposeViewHolder(composeView, viewLifecycleOwner) { - fun bind(message: Message) { - val binding = NimbusMessageCardBinding.bind(itemView) + companion object { + internal val LAYOUT_ID = View.generateViewId() + } - if (message.data.title.isNullOrBlank()) { - binding.titleText.isVisible = false - } else { - binding.titleText.text = message.data.title - } + init { + val horizontalPadding = + composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) + composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) + } - binding.descriptionText.text = message.data.text - - if (message.data.buttonLabel.isNullOrBlank()) { - binding.messageButton.isVisible = false - binding.experimentCard.setOnClickListener { - interactor.onMessageClicked(message) - } - } else { - binding.messageButton.text = message.data.buttonLabel - binding.messageButton.setOnClickListener { - interactor.onMessageClicked(message) - } + @Composable + override fun Content() { + val messaging = components.appStore.observeAsComposableState { state -> state.messaging } + val message = messaging.value?.messageToShow + + message?.let { + MessageCard( + message = it, + onClick = { interactor.onMessageClicked(message) }, + onCloseButtonClick = { interactor.onMessageClosedClicked(message) } + ) } - binding.close.apply { - increaseTapArea(CLOSE_BUTTON_EXTRA_DPS) - setOnClickListener { - interactor.onMessageClosedClicked(message) - } + if (message != null) { + interactor.onMessageDisplayed(message) } - interactor.onMessageDisplayed(message) - } - - companion object { - internal const val LAYOUT_ID = R.layout.nimbus_message_card - private const val CLOSE_BUTTON_EXTRA_DPS = 38 } } diff --git a/app/src/main/res/layout/nimbus_message_card.xml b/app/src/main/res/layout/nimbus_message_card.xml deleted file mode 100644 index 964088bb37..0000000000 --- a/app/src/main/res/layout/nimbus_message_card.xml +++ /dev/null @@ -1,58 +0,0 @@ - - - - - - - - - -