From b7c1f8ef6bd56d6b774d1e1f11bfac55cdc62a9d Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Wed, 25 May 2022 14:22:38 -0400 Subject: [PATCH] Revert "For #23966 - Migrate MessageCardViewHolder to Compose" This reverts commit aea6124851023bb25f664fb49591105b0d959221. --- .../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, 103 insertions(+), 265 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt create 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 ca7d2c9ade..cb790c15da 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 = - 3..4 // The messaging framework is not deterministic and could add a +1 to the count + 4..5 // The messaging framework is not deterministic and could add a +1 to the count private val EXPECTED_NUMBER_OF_INFLATION = - 13..14 // The messaging framework is not deterministic and could add a +1 to the count + 14..15 // 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 358b28ae55..00fc885c59 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.mozac_ic_close_20), + painter = painterResource(R.drawable.ic_close), 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 deleted file mode 100644 index b4dbb25716..0000000000 --- a/app/src/main/java/org/mozilla/fenix/compose/MessageCard.kt +++ /dev/null @@ -1,218 +0,0 @@ -/* 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 080fa059d3..0020078ff8 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,11 +293,6 @@ 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) @@ -323,6 +318,7 @@ class SessionControlAdapter( OnboardingToolbarPositionPickerViewHolder.LAYOUT_ID -> OnboardingToolbarPositionPickerViewHolder( view ) + MessageCardViewHolder.LAYOUT_ID -> MessageCardViewHolder(view, interactor) BottomSpacerViewHolder.LAYOUT_ID -> BottomSpacerViewHolder(view) else -> throw IllegalStateException() } @@ -332,7 +328,6 @@ class SessionControlAdapter( when (holder) { is CollectionHeaderViewHolder, is CustomizeHomeButtonViewHolder, - is MessageCardViewHolder, is NoCollectionsMessageViewHolder, is RecentlyVisitedViewHolder, is RecentVisitsHeaderViewHolder, @@ -395,6 +390,9 @@ 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 ce0bfd6092..8af3c3b240 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.compose.runtime.Composable -import androidx.compose.ui.platform.ComposeView -import androidx.lifecycle.LifecycleOwner -import mozilla.components.lib.state.ext.observeAsComposableState +import androidx.core.view.isVisible +import androidx.recyclerview.widget.RecyclerView import org.mozilla.fenix.R -import org.mozilla.fenix.components.components -import org.mozilla.fenix.compose.ComposeViewHolder -import org.mozilla.fenix.compose.MessageCard +import org.mozilla.fenix.databinding.NimbusMessageCardBinding +import org.mozilla.fenix.ext.increaseTapArea +import org.mozilla.fenix.gleanplumb.Message 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( - composeView: ComposeView, - viewLifecycleOwner: LifecycleOwner, - private val interactor: SessionControlInteractor, -) : ComposeViewHolder(composeView, viewLifecycleOwner) { + view: View, + private val interactor: SessionControlInteractor +) : RecyclerView.ViewHolder(view) { - companion object { - internal val LAYOUT_ID = View.generateViewId() - } + fun bind(message: Message) { + val binding = NimbusMessageCardBinding.bind(itemView) - init { - val horizontalPadding = - composeView.resources.getDimensionPixelSize(R.dimen.home_item_horizontal_margin) - composeView.setPadding(horizontalPadding, 0, horizontalPadding, 0) - } + if (message.data.title.isNullOrBlank()) { + binding.titleText.isVisible = false + } else { + binding.titleText.text = message.data.title + } - @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.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) + } } - if (message != null) { - interactor.onMessageDisplayed(message) + binding.close.apply { + increaseTapArea(CLOSE_BUTTON_EXTRA_DPS) + setOnClickListener { + interactor.onMessageClosedClicked(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 new file mode 100644 index 0000000000..964088bb37 --- /dev/null +++ b/app/src/main/res/layout/nimbus_message_card.xml @@ -0,0 +1,58 @@ + + + + + + + + + +