Fix#4881: Build UI for the Android NPS Survey

[adhiamboperes]

Explanation

Fixes #4881.

This is PR 3 of 6 that adds the UI for the NPS survey as well as completes the key user flows.

Layouts Introduced

Main Layouts

• Survey intro
• Survey fragment for displaying the questions
• Exit confirmation dialog
• Thank you screen

Technical decisions

I used the BindableAdapter.MultiTypeBuilder from within the fragment to populate the views. This allowed me to reuse the survey question layout while creating individual layouts for each answer option group.

I created a viewmodel hierachy where:

• The root fragment viewmodel is responsible for retrieving the correct question text, making the decision to allow navigation to the next question by enabling/disabling the next button based on selected answer availability, and updating the progressbar percentage and progress.
• For the answer option layouts, I created a Super-class, SurveyAnswerItemViewModel for representing the different types of different layouts that can exist for the recyclerView responsible for displaying these views.
• Each question option layout has it's own viewmodel that is responsible for retrieving the answer options to display from the app strings file, notifying that an answer has been provided, and retrieving the selected answer for handling.

I used view binding to display all the strings and control navigation.

I also created custom views where using regular views proved to be complex to work with.

Custom Views

• SurveyOnboardingBackgroundView - for creating the custom background needed on the intro and outro layouts
• SurveyMultipleChoiceOptionView - for binding lists in multiple choice questions
• SurveyNpsItemOptionView - for binding the custom layout needed for the NPS score question

Navigation and Progress

I found it difficult to decouple the UI development work from the progress work, since the survey and the questions are generated dynaically so I needed to develop both in tandem.

The following changes pertain to the core survey logic:

SurveyController

Creates a survey with a list of questions.

SurveyProgressController

Tracks the non-persisted progress of a survey and is responsible for handling the survey session, managing the commands for navigating through and answer submission.

I opted to use a command pattern similar to what is used in the ExplorationProgressController.

Navigation

The survey can be viewed as a series of linear questions, which we will navigate through using methods defined in the SurveyQuestionDeck, modeled after the StateDeck. To start with, we will create an EphemeralSurveyQuestion which represents the question the user is currently viewing. It contains a reference to the next and previous questions if applicable, and a reference to whether it is the last question in the deck. The SurveyQuestionDeck will define the conditions for navigating to the previous and next questions as well as the exceptions thrown in case a navigation action is invalid.

A SurveyQuestionGraph class will be created to provide functionality for processing the behavior of a ‘next’ navigation action, which evaluates which question is shown next.

Survey Proto Changes:

• Refactored the order of the enum items in the UserTypeAnswer enum so that they result into a properly indexed options list per the mocks.
• Modified the EphemeralSurveyQuestion to have a current_question_index and a total_question_count to make it easy to compute the survey progress.
• Added answer typecase free_form_answer to the SurveySelectedAnswer message to hold the value of a free text response.
• Added SurveyQuestion optional_question = 3; field to the survey so that we have a clear seperation between the mandatory and optional questions. This also supports the survey generation architecture.

Bazel

Created BUILD definitions for all new classes and tests.

Dagger

I changed approximately 150 test files to include SurveyQuestionModule::class, though I removed it in a subsequent refactor. When I removed the module and it's usages, it was not a very clean revert because some of the files were not properly formatted before, but they are now -- hence some test files here with only formatting changes.

Usages, Future Work and Out of Scope

The domain layer of the survey is pretty much flexible to support any survey question, including should the current list be reordered or the number of questions changed. For example, if we wanted to show a subsequent survey on the same profile:

Show One Question Only

We would be able to show only one question if we only passed that one question name to the list in the startSurveySession() function. We would also need to set showOptionalQuestion to false, to disable adding the free-form question to the survey. This is otherwise created by default as showOptionalQuestion is always true otherwise.

Further work would need to be done in the UI to to update the navigation button behaviour, i.e currently the button will show next instead of submit, and to ensure the submit answer functionality works to exit survey once the one answer has been submitted.

Show Two or More Questions

We could for example want to show only the NPS score question and the feedback question. We would need to pass in the NPS question's name as an input to the startSurveySession() function. The feedback question name never needs to be explicitly passed since we create a default (promoter) question and typically update the actual question based on the NPS score.

The UI currently supports this scenario.

Add More Question Types

The domain layer would be able to handle any question types, but the UI will need to be updated to display this question by creating a new layout, and viewmodel for it, as well as updating the SurveyAnswerItemViewModel's ViewType enum with this new viewType(s).

A more longterm solution for a dynamic UI would be to create standardized survey questions and answer options, pretty much like state/exploration interactions.

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

• Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
• For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide]

LTR	RTL
https://github.com/oppia/oppia-android/assets/59600948/d2411422-aa99-482d-a0e7-c6fd163e85e1	https://github.com/oppia/oppia-android/assets/59600948/329e0672-8d4c-4f43-8c6c-b9036a78980a
Updated LTR video with restored answers
https://github.com/oppia/oppia-android/assets/59600948/1a0decff-7dbf-4a6a-a225-5f38f4191fd6

[adhiamboperes]

Thanks @adhiamboperes! I left a few inline comments, PTAL. Sorry that you had to edit so many files because of the Dagger thing :(
Thanks, I think this is one of the problems I want to solve in the coming weeks.
I also took a look at the videos. They look great! Some comments:

• The multiple-choice option texts are a bit out-of-alignment vertically from the radio buttons. Could that be fixed?

Thanks for catching this, I hadn't realized. Yes, I can definitely fix this.

• Are the user's previous answers saved, and if so, could they be shown on back-navigation (the user can, of course, still change them)? It seems odd to me that, when going back to a previous survey question, the original answer has disappeared. It feels like the user's answer wasn't saved at all.

Yes, we do save them--I had put this logic in a different branch that handles saving and logging, but I can specifically cherry-pick it here if it makes better sense.

• Could you show what happens when the user navigates back two questions and then forward one question?

I'll be adding a video and a UI test for that.

• Could you show verification for when the survey completion events were registered and what data they contain (or is that going to be a different PR)?

Yes, this is in the next PR in the chain.

Thank you for taking a look!

Hi @seanlip,

I have addressed all your comments on the PR, PTAL!

I have also updated the PR description and the videos.

[adhiamboperes]

This PR has an existing test file that is failing on bazel but passing on gradle, and I am not sure why, yet. The changes in this PR do not relate to Checkpointing at all. I however do not think this should continue blocking review, and I am going to keep looking into it.

The only failure I expect that will be fixed by @MohitGupta121 in PR #5076 is the static check related to #5072.

This turned out to be a flaky test scenario, they are mostly passing.

[oppiabot]

Unassigning @adhiamboperes since a re-review was requested. @adhiamboperes, please make sure you have addressed all review comments. Thanks!

[seanlip]

Thanks @adhiamboperes! Just a few more inline comments. Additionally, for the new video:

• At 0:07 and 0:10 it seems that an incomplete question displays and then later the options appear. This is quite jarring. Can all parts of the question be loaded at once?
• Around 0:30 there is a brief flash of the textarea and then it jumps back to the NPS question. This seems like a bug?

[adhiamboperes]

I have looked into these:

• At 0:07 and 0:10 it seems that an incomplete question displays and then later the options appear. This is quite jarring. Can all parts of the question be loaded at once?

This is only visible because I had recorded the video at an extremely low bitrate(4Mbps). In reality, I am not able to observe serious visual jank.

• Around 0:30 there is a brief flash of the textarea and then it jumps back to the NPS question. This seems like a bug?

I rechecked and this happened between actual next and prev button clicks in quick succession. I cannot repro this.

1. I have re-recorded the video with standard video settings.
2. I have rechecked the code and could not find logic breaks to cause the text area question to load unless navigated to explicitly.

[seanlip]

Thanks @adhiamboperes! LGTM.

Just one thing I noticed when watching the demo again -- the fourth bullet point in the second question should be "N/A - I don't use Oppia anymore". I.e. there is a missing "I".

[oppiabot]

Hi @adhiamboperes, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

[adhiamboperes]

Thanks @adhiamboperes! LGTM.

Just one thing I noticed when watching the demo again -- the fourth bullet point in the second question should be "N/A - I don't use Oppia anymore". I.e. there is a missing "I".

I am curious about this: from the mocks there is no I, but other PMF samples I have seen seem to have it.

[seanlip]

I am curious about this: from the mocks there is no I, but other PMF samples I have seen seem to have it.

Yup, I think that was just a mistake, let's please add it.

(Also in the future, if you see anything that looks "off" please do bring it up directly with the design/product team. Thanks!)

[kkmurerwa]

Hey @adhiamboperes. I have reviewed the remaining test issue and that is now resolved. Everything is good now.

[adhiamboperes]

Thanks @seanlip, @kkmurerwa @MohitGupta121!