Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed feedback link from reading preferences #21466

Merged
merged 5 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,23 +20,18 @@ 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) {
private val originalReadingPreferences = getReadingPreferences()
private val _currentReadingPreferences = MutableStateFlow(originalReadingPreferences)
val currentReadingPreferences: StateFlow<ReaderReadingPreferences> = _currentReadingPreferences

private val _isFeedbackEnabled = MutableStateFlow(false)
val isFeedbackEnabled: StateFlow<Boolean> = _isFeedbackEnabled

private val _actionEvents = MutableSharedFlow<ActionEvent>()
val actionEvents: SharedFlow<ActionEvent> = _actionEvents

fun init() {
launch {
_isFeedbackEnabled.emit(readingPreferencesFeedbackFeatureConfig.isEnabled())
_actionEvents.emit(ActionEvent.UpdateStatusBarColor(originalReadingPreferences.theme))
}
}
Expand Down Expand Up @@ -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()) {
Expand All @@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +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.LinkAnnotation
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
Expand All @@ -63,19 +57,15 @@ 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
Expand All @@ -87,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
Expand Down Expand Up @@ -145,14 +135,6 @@ fun ReadingPreferencesScreen(
style = contentStyle,
)

if (isFeedbackEnabled) {
ReadingPreferencesPreviewFeedback(
onSendFeedbackClick = onSendFeedbackClick,
textStyle = contentStyle,
linkColor = linkColor,
)
}

// Tags
FlowRow(
modifier = Modifier.fillMaxWidth(),
Expand Down Expand Up @@ -195,7 +177,7 @@ fun ReadingPreferencesScreen(
theme = theme,
isSelected = theme == currentReadingPreferences.theme,
onClick = {
haptics?.performHapticFeedback(HapticFeedbackType.LongPress)
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
onThemeClick(theme)
},
)
Expand All @@ -218,7 +200,7 @@ fun ReadingPreferencesScreen(
fontFamily = fontFamily,
isSelected = fontFamily == currentReadingPreferences.fontFamily,
onClick = {
haptics?.performHapticFeedback(HapticFeedbackType.LongPress)
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
onFontFamilyClick(fontFamily)
},
)
Expand All @@ -235,7 +217,7 @@ fun ReadingPreferencesScreen(
previewFontFamily = fontFamily,
selectedFontSize = currentReadingPreferences.fontSize,
onFontSizeSelected = {
haptics?.performHapticFeedback(HapticFeedbackType.LongPress)
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
onFontSizeClick(it)
},
)
Expand All @@ -260,55 +242,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,
)

addLink(
clickable = LinkAnnotation.Clickable(
tag = "url",
linkInteractionListener = {
onSendFeedbackClick()
}
),
start = startIndex,
end = endIndex,
)
}

val buttonLabel = stringResource(R.string.reader_preferences_screen_preview_text_feedback_label)
Text(
text = annotatedString,
style = textStyle,
modifier = Modifier.semantics {
onClick(
label = buttonLabel,
) {
onSendFeedbackClick()
true
}
},
)
}

private fun getTitleTextStyle(
fontFamily: FontFamily,
Expand All @@ -335,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 = {},
)
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -31,9 +30,6 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() {
@Mock
lateinit var saveReadingPreferences: ReaderSaveReadingPreferencesUseCase

@Mock
lateinit var readingPreferencesFeedbackFeatureConfig: ReaderReadingPreferencesFeedbackFeatureConfig

@Mock
lateinit var readingPreferencesTracker: ReaderReadingPreferencesTracker

Expand All @@ -49,7 +45,6 @@ class ReaderReadingPreferencesViewModelTest : BaseUnitTest() {
viewModel = ReaderReadingPreferencesViewModel(
getReadingPreferences,
saveReadingPreferences,
readingPreferencesFeedbackFeatureConfig,
readingPreferencesTracker,
viewModelDispatcher,
)
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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"
}
}
Loading