Fix #159: Introduce question player UI structure #500
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
empty test suite.
… training controller
…ler-interface Conflicts: domain/src/main/java/org/oppia/domain/question/QuestionAssessmentProgressController.kt domain/src/main/java/org/oppia/domain/question/QuestionTrainingController.kt domain/src/test/java/org/oppia/domain/question/QuestionTrainingControllerTest.kt model/src/main/proto/question.proto
This includes some basic refactoring of internal structures used by the exploration progress controller to share common functionality between the two progress controllers. There's still some duplication, but this seems like a reasonable split since there's likely to be further differences in the progress controllers in the future. The question assessment progress controller tests pass, but no new ones have yet been added to thoroughly test the implementation.
ExplorationProgressController. They haven't yet been verified as correct.
…terface Conflicts: domain/src/main/assets/sample_questions.json domain/src/main/java/org/oppia/domain/question/QuestionAssessmentProgressController.kt domain/src/main/java/org/oppia/domain/question/QuestionTrainingController.kt domain/src/main/java/org/oppia/domain/util/JsonAssetRetriever.kt domain/src/test/java/org/oppia/domain/question/QuestionTrainingControllerTest.kt
|
Thanks all! @veena14cs sending this back to you to confirm whether I addressed your comments. Also and also to you @rt4914 for your one comment. |
…-progress-controller-implementation
|
Unassigned myself as I have resolved all comments. |
Base automatically changed from
introduce-question-progress-controller-implementation
to
develop
May 28, 2020 18:21
…into introduce-question-player-ui-structure
BenHenning
changed the title
veena14cs
suggested changes
Jun 8, 2020
behavior graph (and added an actual visual representation of this graph in code to simplify maintaining the solution in the future). Tests will be added as part of solving #1273.
|
@veena14cs & @nikitamarysolomanpvt sending this back to you to resolve your open comment threads. Please let me know if you have any questions. |
Conflicts: app/src/main/java/org/oppia/app/application/ApplicationComponent.kt app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #159.
This introduces full support for the question player, minus early exit support (which will be a follow-up work item post-prototype).
This PR is primarily refactoring the state fragment presenter into a shared StatePlayerRecyclerViewAssembler object that can be used to arrange the recycler views for both explorations and questions. This is done in a compositional way that facilitates future features being added, or disparity in functionality between the two (e.g. no previous button in questions and no replay button in explorations).
This PR also includes reworking how the GCS resource bucket name is provided to be normalized (we were using two different annotations before), and set up such that we can change the bucket used depending on context (questions require the test server bucket rather than the production one).
#503 tracks adding thorough tests for the question player once #89 is resolved. This wasn't done as part of this PR for a few reasons:
This replaces #411 by building on the work started by @jamesxu0.
Open issues that should be resolved after this PR is done: