Fix #4652 : Implemented Learner Progress for Exploration State

Rd4dev commented 5 months ago

Fixes #4652

Explanation

This PR provides an implementation for displaying learning progress in the exploration lessons.
As mentioned in the issue, Oppia lessons are dynamic in nature, and to define the progress, the flow of lessons is mandatory.
- One way we could fix this is by providing order numbers to the JSON files themselves, but that would make it difficult to scale and add other states in the future.
- So, I implemented a small parser that identifies the order of the lessons by going through the JSON.

Screenshot (450)

The way the parser works is as follows:
- It starts off with init_state_name.
- With each state, it first identifies its default_outcome and assigns a position to them.
- Then, it checks the answers_group from bottom to top and assigns those states their positions one by one (note that only the first entries are noted).
- Thereby, the order of the exploration lessons is obtained. I have attached the pseudocode for reference and the obtained list of statename : positions below.

exploration_pseudo_code

The way it handles the dynamic changes of questions is that the total number of questions remains the same as the total number of states. The current state is checked on every new exploration. Let's say we have 3 states;
- answering state 1 correctly will skip you to state 3, making the progress jump from 10% to 30%.
- If state 1 is answered incorrectly, the result_answers are checked, and state 2 is triggered, making the progress go from 10% to 20%.
- The same applies to the decrement of progress when recapping back to previous states.
The progress bar uses the same design as the question player progress bar.

TODO:

runBlocking should be properly checked and handled if needed, replaced with proper coroutine.
Tests will be updated soon. -[Hindered Implementation due to Bug #5379
Check landscape implementation
To check visibility and proper positioning with audio block
Note:
This is a proposed solution. I would love to hear feedback on whether this would be viable or not.

Essential Checklist

[x] 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: ...".)
[x] Any changes to scripts/assets files have their rationale included in the PR explanation.
[x] The PR follows the style guide.
[x] The PR does not contain any unnecessary code changes from Android Studio (reference).
[x] The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
[x] The PR is assigned to the appropriate reviewers (reference).

Demonstration

https://github.com/oppia/oppia-android/assets/122200035/1ac1b67a-6ab0-4b81-9511-23759a86b244

Rd4dev commented 5 months ago

@BenHenning, @adhiamboperes Would this approach be effective?

oppiabot[bot] commented 5 months ago

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

Rd4dev commented 5 months ago

@adhiamboperes PTAL

seanlip commented 5 months ago

THis is quite interesting, thanks @Rd4dev!

From the product side, I don't have an objection, it would actually not be a bad idea to test this. The only request I have is to put this functionality behind a feature flag so that we can do some controlled testing before launching it more broadly.

The only other concern I have is that it's a bit weird how the content gets "sucked up" into the progress bar -- I'm not sure about the handling of the cutoff when scrolling. Would you mind filing an issue on the design team repo to take a look at this, and I will talk to the Android design team to see if they can look at it? This doesn't need to block the merge of this PR though (assuming that the functionality remains gated behind a feature flag).

Thanks!

Rd4dev commented 5 months ago

Thank you for the initial approval, @seanlip, I've filed a new issue in the design team repository. Will reserve the feature under a feature flag in the upcoming commits.

oppiabot[bot] commented 4 months ago

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot[bot] commented 4 months ago

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

BenHenning commented 4 months ago

@Rd4dev are you planning on working on this? Otherwise, I think it might be best if we close it and unassign the issue so that others can take it up if they have availability.

It sounds based on Sean's latest feedback that we'd need to update the import asset script & domain protos for lessons to pull in the checkpoint properties so that we have a baseline for computing progress through an exploration.

Rd4dev commented 4 months ago

@BenHenning I don't think I will be able to work on this right now. I was initially looking into pathfinding algorithm implementations, but after Sean mentioned the necessary asset updates, I think it's best to unassign myself for now. I'll let someone else take over, and if it's still available later, I'll pick it up when I can. For now, I'll unassign myself. Thanks for the ping.

oppiabot[bot] commented 4 months ago

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppia / oppia-android