From 47e9c077b8a63495b1a987cb6de98b89a3589613 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Thu, 14 Nov 2024 13:51:50 -0500 Subject: [PATCH 1/3] Removed feedback link from reading preferences --- .../ReaderReadingPreferencesDialogFragment.kt | 3 - .../ReaderReadingPreferencesTracker.kt | 4 - .../ReaderReadingPreferencesViewModel.kt | 17 ----- .../ReadingPreferencesScreen.kt | 75 ------------------- ...ReadingPreferencesFeedbackFeatureConfig.kt | 21 ------ .../android/analytics/AnalyticsTracker.java | 1 - 6 files changed, 121 deletions(-) delete mode 100644 WordPress/src/main/java/org/wordpress/android/util/config/ReaderReadingPreferencesFeedbackFeatureConfig.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderReadingPreferencesDialogFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderReadingPreferencesDialogFragment.kt index 6c94e888d2e0..56918efa5174 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderReadingPreferencesDialogFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderReadingPreferencesDialogFragment.kt @@ -60,16 +60,13 @@ class ReaderReadingPreferencesDialogFragment : BottomSheetDialogFragment() { setContent { AppThemeM3 { val readerPreferences by viewModel.currentReadingPreferences.collectAsState() - val isFeedbackEnabled by viewModel.isFeedbackEnabled.collectAsState() ReadingPreferencesScreen( currentReadingPreferences = readerPreferences, onCloseClick = viewModel::onExitActionClick, - onSendFeedbackClick = viewModel::onSendFeedbackClick, onThemeClick = viewModel::onThemeClick, onFontFamilyClick = viewModel::onFontFamilyClick, onFontSizeClick = viewModel::onFontSizeClick, onBackgroundColorUpdate = { dialog?.window?.setWindowStatusBarColor(it) }, - isFeedbackEnabled = isFeedbackEnabled, ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTracker.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTracker.kt index 6dba0899bdee..6f22992eecf1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTracker.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTracker.kt @@ -18,10 +18,6 @@ class ReaderReadingPreferencesTracker @Inject constructor( analyticsTrackerWrapper.track(AnalyticsTracker.Stat.READER_READING_PREFERENCES_CLOSED) } - fun trackFeedbackTapped() { - analyticsTrackerWrapper.track(AnalyticsTracker.Stat.READER_READING_PREFERENCES_FEEDBACK_TAPPED) - } - fun trackItemTapped(theme: ReaderReadingPreferences.Theme) { val props = mapOf( PROP_TYPE_KEY to PROP_TYPE_THEME, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModel.kt index 2865b81c803c..1391687aa35d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModel.kt @@ -12,7 +12,6 @@ import org.wordpress.android.ui.reader.models.ReaderReadingPreferences import org.wordpress.android.ui.reader.tracker.ReaderReadingPreferencesTracker import org.wordpress.android.ui.reader.usecases.ReaderGetReadingPreferencesSyncUseCase import org.wordpress.android.ui.reader.usecases.ReaderSaveReadingPreferencesUseCase -import org.wordpress.android.util.config.ReaderReadingPreferencesFeedbackFeatureConfig import org.wordpress.android.viewmodel.ScopedViewModel import javax.inject.Inject import javax.inject.Named @@ -21,7 +20,6 @@ import javax.inject.Named class ReaderReadingPreferencesViewModel @Inject constructor( getReadingPreferences: ReaderGetReadingPreferencesSyncUseCase, private val saveReadingPreferences: ReaderSaveReadingPreferencesUseCase, - private val readingPreferencesFeedbackFeatureConfig: ReaderReadingPreferencesFeedbackFeatureConfig, private val readingPreferencesTracker: ReaderReadingPreferencesTracker, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, ) : ScopedViewModel(bgDispatcher) { @@ -29,15 +27,11 @@ class ReaderReadingPreferencesViewModel @Inject constructor( private val _currentReadingPreferences = MutableStateFlow(originalReadingPreferences) val currentReadingPreferences: StateFlow = _currentReadingPreferences - private val _isFeedbackEnabled = MutableStateFlow(false) - val isFeedbackEnabled: StateFlow = _isFeedbackEnabled - private val _actionEvents = MutableSharedFlow() val actionEvents: SharedFlow = _actionEvents fun init() { launch { - _isFeedbackEnabled.emit(readingPreferencesFeedbackFeatureConfig.isEnabled()) _actionEvents.emit(ActionEvent.UpdateStatusBarColor(originalReadingPreferences.theme)) } } @@ -92,13 +86,6 @@ class ReaderReadingPreferencesViewModel @Inject constructor( } } - fun onSendFeedbackClick() { - launch { - readingPreferencesTracker.trackFeedbackTapped() - _actionEvents.emit(ActionEvent.OpenWebView(FEEDBACK_URL)) - } - } - private suspend fun saveReadingPreferencesInternal() { val currentPreferences = currentReadingPreferences.value if (isDirty()) { @@ -115,8 +102,4 @@ class ReaderReadingPreferencesViewModel @Inject constructor( data class UpdateStatusBarColor(val theme: ReaderReadingPreferences.Theme) : ActionEvent data class OpenWebView(val url: String) : ActionEvent } - - companion object { - private const val FEEDBACK_URL = "https://automattic.survey.fm/reader-customization-survey" - } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt index d42ff89947c8..fe8f999db326 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt @@ -15,7 +15,6 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.wrapContentHeight import androidx.compose.foundation.rememberScrollState -import androidx.compose.foundation.text.ClickableText import androidx.compose.foundation.verticalScroll import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -35,14 +34,9 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalHapticFeedback import androidx.compose.ui.platform.rememberNestedScrollInteropConnection import androidx.compose.ui.res.stringResource -import androidx.compose.ui.semantics.onClick -import androidx.compose.ui.semantics.semantics -import androidx.compose.ui.text.SpanStyle import androidx.compose.ui.text.TextStyle -import androidx.compose.ui.text.buildAnnotatedString import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.text.style.TextDecoration import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.sp import org.wordpress.android.R @@ -63,19 +57,16 @@ private const val TEXT_LINE_HEIGHT_MULTIPLIER = 1.6f fun ReadingPreferencesScreen( currentReadingPreferences: ReaderReadingPreferences, onCloseClick: () -> Unit, - onSendFeedbackClick: () -> Unit, onThemeClick: (ReaderReadingPreferences.Theme) -> Unit, onFontFamilyClick: (ReaderReadingPreferences.FontFamily) -> Unit, onFontSizeClick: (ReaderReadingPreferences.FontSize) -> Unit, onBackgroundColorUpdate: (Int) -> Unit, - isFeedbackEnabled: Boolean, isHapticsFeedbackEnabled: Boolean = true, ) { val themeValues = ReaderReadingPreferences.ThemeValues.from(LocalContext.current, currentReadingPreferences.theme) val backgroundColor by animateColorAsState(Color(themeValues.intBackgroundColor), label = "backgroundColor") val baseTextColor by animateColorAsState(Color(themeValues.intBaseTextColor), label = "baseTextColor") val textColor by animateColorAsState(Color(themeValues.intTextColor), label = "textColor") - val linkColor by animateColorAsState(Color(themeValues.intLinkColor), label = "linkColor") SideEffect { // update background color based on value animation and notify the parent @@ -145,14 +136,6 @@ fun ReadingPreferencesScreen( style = contentStyle, ) - if (isFeedbackEnabled) { - ReadingPreferencesPreviewFeedback( - onSendFeedbackClick = onSendFeedbackClick, - textStyle = contentStyle, - linkColor = linkColor, - ) - } - // Tags FlowRow( modifier = Modifier.fillMaxWidth(), @@ -260,62 +243,6 @@ private fun ExperimentalBadge( ) } -@Composable -private fun ReadingPreferencesPreviewFeedback( - onSendFeedbackClick: () -> Unit, - textStyle: TextStyle, - linkColor: Color, -) { - val linkString = stringResource(R.string.reader_preferences_screen_preview_text_feedback_link) - val feedbackString = stringResource(R.string.reader_preferences_screen_preview_text_feedback, linkString) - val annotatedString = buildAnnotatedString { - append(feedbackString) - - val startIndex = feedbackString.indexOf(linkString) - val endIndex = startIndex + linkString.length - - addStyle( - style = SpanStyle( - color = linkColor, - textDecoration = TextDecoration.Underline, - ), - start = startIndex, - end = endIndex, - ) - - addStringAnnotation( - tag = "url", - annotation = "feedback", - start = startIndex, - end = endIndex, - ) - } - - val buttonLabel = stringResource(R.string.reader_preferences_screen_preview_text_feedback_label) - @Suppress("DEPRECATION") - ClickableText( - text = annotatedString, - style = textStyle, - onClick = { offset -> - annotatedString.getStringAnnotations(tag = "url", start = offset, end = offset) - .firstOrNull() - ?.let { annotation -> - if (annotation.item == "feedback") { - onSendFeedbackClick() - } - } - }, - modifier = Modifier.semantics { - onClick( - label = buttonLabel, - ) { - onSendFeedbackClick() - true - } - }, - ) -} - private fun getTitleTextStyle( fontFamily: FontFamily, fontSizeMultiplier: Float, @@ -341,11 +268,9 @@ private fun ReadingPreferencesScreenPreview() { ReadingPreferencesScreen( currentReadingPreferences = readingPreferences, onCloseClick = {}, - onSendFeedbackClick = {}, onThemeClick = { readingPreferences = readingPreferences.copy(theme = it) }, onFontFamilyClick = { readingPreferences = readingPreferences.copy(fontFamily = it) }, onFontSizeClick = { readingPreferences = readingPreferences.copy(fontSize = it) }, - isFeedbackEnabled = true, onBackgroundColorUpdate = {}, ) } diff --git a/WordPress/src/main/java/org/wordpress/android/util/config/ReaderReadingPreferencesFeedbackFeatureConfig.kt b/WordPress/src/main/java/org/wordpress/android/util/config/ReaderReadingPreferencesFeedbackFeatureConfig.kt deleted file mode 100644 index 1685455758b6..000000000000 --- a/WordPress/src/main/java/org/wordpress/android/util/config/ReaderReadingPreferencesFeedbackFeatureConfig.kt +++ /dev/null @@ -1,21 +0,0 @@ -package org.wordpress.android.util.config - -import org.wordpress.android.BuildConfig -import org.wordpress.android.annotation.Feature -import javax.inject.Inject - -private const val READING_PREFERENCES_FEEDBACK_REMOTE_FIELD = "reading_preferences_feedback" -@Feature( - READING_PREFERENCES_FEEDBACK_REMOTE_FIELD, - true, -) -class ReaderReadingPreferencesFeedbackFeatureConfig -@Inject constructor(appConfig: AppConfig) : FeatureConfig( - appConfig, - BuildConfig.READER_READING_PREFERENCES_FEEDBACK, - READING_PREFERENCES_FEEDBACK_REMOTE_FIELD, -) { - override fun isEnabled(): Boolean { - return super.isEnabled() && BuildConfig.IS_JETPACK_APP - } -} diff --git a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java index 46efebe88cf2..3cb7f9dac92c 100644 --- a/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java +++ b/libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java @@ -89,7 +89,6 @@ public enum Stat { READER_FOLLOWING_FETCHED, READER_READING_PREFERENCES_OPENED, READER_READING_PREFERENCES_CLOSED, - READER_READING_PREFERENCES_FEEDBACK_TAPPED, READER_READING_PREFERENCES_ITEM_TAPPED, READER_READING_PREFERENCES_SAVED, READER_ANNOUNCEMENT_CARD_DISMISSED, From 61bd3815227a7441013d0687b2181ddfae7d11f7 Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 15 Nov 2024 07:30:50 -0500 Subject: [PATCH 2/3] Removed unnecessary parameter --- .../readingpreferences/ReadingPreferencesScreen.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt index fe8f999db326..7d9b6b33e554 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/readingpreferences/ReadingPreferencesScreen.kt @@ -61,7 +61,6 @@ fun ReadingPreferencesScreen( onFontFamilyClick: (ReaderReadingPreferences.FontFamily) -> Unit, onFontSizeClick: (ReaderReadingPreferences.FontSize) -> Unit, onBackgroundColorUpdate: (Int) -> Unit, - isHapticsFeedbackEnabled: Boolean = true, ) { val themeValues = ReaderReadingPreferences.ThemeValues.from(LocalContext.current, currentReadingPreferences.theme) val backgroundColor by animateColorAsState(Color(themeValues.intBackgroundColor), label = "backgroundColor") @@ -78,7 +77,7 @@ fun ReadingPreferencesScreen( val fontSize = currentReadingPreferences.fontSize.toSp() val fontSizeMultiplier = currentReadingPreferences.fontSize.multiplier - val haptics = LocalHapticFeedback.current.takeIf { isHapticsFeedbackEnabled } + val haptics = LocalHapticFeedback.current Column( modifier = Modifier @@ -178,7 +177,7 @@ fun ReadingPreferencesScreen( theme = theme, isSelected = theme == currentReadingPreferences.theme, onClick = { - haptics?.performHapticFeedback(HapticFeedbackType.LongPress) + haptics.performHapticFeedback(HapticFeedbackType.LongPress) onThemeClick(theme) }, ) @@ -201,7 +200,7 @@ fun ReadingPreferencesScreen( fontFamily = fontFamily, isSelected = fontFamily == currentReadingPreferences.fontFamily, onClick = { - haptics?.performHapticFeedback(HapticFeedbackType.LongPress) + haptics.performHapticFeedback(HapticFeedbackType.LongPress) onFontFamilyClick(fontFamily) }, ) @@ -218,7 +217,7 @@ fun ReadingPreferencesScreen( previewFontFamily = fontFamily, selectedFontSize = currentReadingPreferences.fontSize, onFontSizeSelected = { - haptics?.performHapticFeedback(HapticFeedbackType.LongPress) + haptics.performHapticFeedback(HapticFeedbackType.LongPress) onFontSizeClick(it) }, ) From e5f5734be81ff20feafb271592079a1b197dbdaf Mon Sep 17 00:00:00 2001 From: Nick Bradbury Date: Fri, 15 Nov 2024 15:10:32 -0500 Subject: [PATCH 3/3] Fixed failing tests --- .../ReaderReadingPreferencesTrackerTest.kt | 7 --- .../ReaderReadingPreferencesViewModelTest.kt | 59 ++----------------- 2 files changed, 4 insertions(+), 62 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTrackerTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTrackerTest.kt index 9a7593a8a5ed..59dd78e283e4 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTrackerTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/tracker/ReaderReadingPreferencesTrackerTest.kt @@ -48,13 +48,6 @@ class ReaderReadingPreferencesTrackerTest { verify(analyticsTrackerWrapper).track(Stat.READER_READING_PREFERENCES_CLOSED) } - @Test - fun `when trackFeedbackTapped is called, then track event`() { - tracker.trackFeedbackTapped() - - verify(analyticsTrackerWrapper).track(Stat.READER_READING_PREFERENCES_FEEDBACK_TAPPED) - } - @Test fun `when trackItemTapped is called with theme, then track event`() { ReaderReadingPreferences.Theme.values().forEach { theme -> diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModelTest.kt index 9a5e8a3e8865..782083efee8b 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderReadingPreferencesViewModelTest.kt @@ -21,7 +21,6 @@ import org.wordpress.android.ui.reader.tracker.ReaderReadingPreferencesTracker import org.wordpress.android.ui.reader.usecases.ReaderGetReadingPreferencesSyncUseCase import org.wordpress.android.ui.reader.usecases.ReaderSaveReadingPreferencesUseCase import org.wordpress.android.ui.reader.viewmodels.ReaderReadingPreferencesViewModel.ActionEvent -import org.wordpress.android.util.config.ReaderReadingPreferencesFeedbackFeatureConfig @ExperimentalCoroutinesApi class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { @@ -31,9 +30,6 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { @Mock lateinit var saveReadingPreferences: ReaderSaveReadingPreferencesUseCase - @Mock - lateinit var readingPreferencesFeedbackFeatureConfig: ReaderReadingPreferencesFeedbackFeatureConfig - @Mock lateinit var readingPreferencesTracker: ReaderReadingPreferencesTracker @@ -49,7 +45,6 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { viewModel = ReaderReadingPreferencesViewModel( getReadingPreferences, saveReadingPreferences, - readingPreferencesFeedbackFeatureConfig, readingPreferencesTracker, viewModelDispatcher, ) @@ -211,46 +206,10 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { assertThat(updateEvent).isEqualTo(ActionEvent.UpdatePostDetails) } - @Test - fun `when onSendFeedbackClick is called then it emits OpenWebView action event`() = test { - // When - viewModel.onSendFeedbackClick() - - // Then - val openWebViewEvent = collectedEvents.last() as ActionEvent.OpenWebView - assertThat(openWebViewEvent.url).isEqualTo(EXPECTED_FEEDBACK_URL) - } - - @Test - fun `when readerReadingPreferencesFeedbackFeatureConfig is true then isFeedbackEnabled emits true`() = test { - // Given - whenever(readingPreferencesFeedbackFeatureConfig.isEnabled()).thenReturn(true) - - // When - viewModel.init() - - // Then - val isFeedbackEnabled = viewModel.isFeedbackEnabled.first() - assertThat(isFeedbackEnabled).isTrue() - } - - @Test - fun `when readerReadingPreferencesFeedbackFeatureConfig is false then isFeedbackEnabled emits false`() = test { - // Given - whenever(readingPreferencesFeedbackFeatureConfig.isEnabled()).thenReturn(false) - - // When - viewModel.init() - - // Then - val isFeedbackEnabled = viewModel.isFeedbackEnabled.first() - assertThat(isFeedbackEnabled).isFalse() - } - // analytics tests @Test fun `when onScreenOpened is called then it should track the screen opened event`() = test { - ReaderReadingPreferencesTracker.Source.values().forEach { source -> + ReaderReadingPreferencesTracker.Source.entries.forEach { source -> // When viewModel.onScreenOpened(source) @@ -268,18 +227,9 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { verify(readingPreferencesTracker).trackScreenClosed() } - @Test - fun `when onSendFeedbackClick is called then it should track the feedback tapped event`() = test { - // When - viewModel.onSendFeedbackClick() - - // Then - verify(readingPreferencesTracker).trackFeedbackTapped() - } - @Test fun `when onThemeClick is called then it should track the theme tapped event`() = test { - ReaderReadingPreferences.Theme.values().forEach { theme -> + ReaderReadingPreferences.Theme.entries.forEach { theme -> // When viewModel.onThemeClick(theme) @@ -290,7 +240,7 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { @Test fun `when onFontFamilyClick is called then it should track the font family tapped event`() = test { - ReaderReadingPreferences.FontFamily.values().forEach { fontFamily -> + ReaderReadingPreferences.FontFamily.entries.forEach { fontFamily -> // When viewModel.onFontFamilyClick(fontFamily) @@ -301,7 +251,7 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { @Test fun `when onFontSizeClick is called then it should track the font size tapped event`() = test { - ReaderReadingPreferences.FontSize.values().forEach { fontSize -> + ReaderReadingPreferences.FontSize.entries.forEach { fontSize -> // When viewModel.onFontSizeClick(fontSize) @@ -325,6 +275,5 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() { companion object { private val DEFAULT_READING_PREFERENCES = ReaderReadingPreferences() - private const val EXPECTED_FEEDBACK_URL = "https://automattic.survey.fm/reader-customization-survey" } }