Yir Survey #6001
Yir Survey #6001
Conversation
This reverts commit 42f85eb.
# Conflicts: # app/src/main/java/org/wikipedia/yearinreview/YearInReviewActivity.kt # app/src/main/java/org/wikipedia/yearinreview/YearInReviewScreenDeck.kt
| var showYearInReviewSurvey | ||
| get() = PrefsIoUtil.getBoolean(R.string.preference_key_show_yir_survey, false) | ||
| set(value) = PrefsIoUtil.setBoolean(R.string.preference_key_show_yir_survey, value) |
There was a problem hiding this comment.
I feel like we should have this saved in the YearInReviewModel, otherwise we will not be able to show the survey for next year's YiR.
There was a problem hiding this comment.
Do you mean this data class?
@serializable
data class YearInReviewModel(
val totalReadingTimeMinutes: Long,
.....
)
There was a problem hiding this comment.
Yes, because the survey should be shown every year
| if (!Prefs.showYearInReviewSurvey || Prefs.yearInReviewSurveyShown) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Not sure, but would it be possible to reduce one variable like shouldShowYiRSurvey?
There was a problem hiding this comment.
I added showYearInReviewSurvey to allow triggering the survey either after the user has viewed more than two slides or from the end of the highlights screen. This is needed because I am calling maybeShowYearInReviewFeedbackDialog from onResume in both MainFragment and PageActivity. The yearInReviewSurveyShown flag is used to ensure the survey is only shown once.
There was a problem hiding this comment.
Ah I see. Maybe these two preferences can be saved in the YearInReviewModel too.
# Conflicts: # app/src/main/java/org/wikipedia/page/PageActivity.kt
|
Instead of adding new variables to the model data (which is a map) and managing get/set operations across multiple places in the code, which would add lots of extra code, would it be simpler to just reset the survey preference when a new Year in Review loads in YearInReviewViewModel? |
# Conflicts: # app/src/main/java/org/wikipedia/main/MainFragment.kt # app/src/main/java/org/wikipedia/page/PageActivity.kt # app/src/main/java/org/wikipedia/yearinreview/YearInReviewActivity.kt # app/src/main/java/org/wikipedia/yearinreview/YearInReviewScreenDeck.kt # app/src/main/java/org/wikipedia/yearinreview/YearInReviewViewModel.kt # app/src/main/res/values/preference_keys.xml
- adds list preference as developer settings for year in review survey - code fixes
|
Please resolve the conflict file. |
# Conflicts: # app/src/main/java/org/wikipedia/page/PageActivity.kt # app/src/main/java/org/wikipedia/yearinreview/YearInReviewViewModel.kt
app/src/main/java/org/wikipedia/yearinreview/YearInReviewActivity.kt
Outdated
Show resolved
Hide resolved
| }, | ||
| onNextButtonClick = { pagerState -> | ||
| onNextButtonClick = { pagerState, currentSlideData -> | ||
| viewModel.slideViewedCount += 1 |
There was a problem hiding this comment.
Can we have the following logic in onNextButtonClick so we do not need to put it in the onBackButtonClick?
if (viewModel.slideViewedCount >= 2 && Prefs.yearInReviewSurveyState == YearInReviewSurveyState.NOT_TRIGGERED) {
Prefs.yearInReviewSurveyState = YearInReviewSurveyState.SHOULD_SHOW
}
There was a problem hiding this comment.
The logic in onBackButtonClick is for this requirement.(or After someone has viewed 2 or more slides closed year in Review from any slide). I renamed the onBackButtonClick to onCloseButtonClick.
3 participants
What does this do?
Phabricator:
https://phabricator.wikimedia.org/T405504