-
Notifications
You must be signed in to change notification settings - Fork 527
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
Fixes #160: Integrating topic controller into Concept Card #198
Conversation
have yet been added.
…ully working yet.
…view-adapter' into concept-card-topic-controller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app/src/main/java/org/oppia/app/recyclerview/BindableAdapter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/app/topic/conceptcard/ConceptCardFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/app/topic/conceptcard/ConceptCardPresenter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/app/topic/conceptcard/testing/ConceptCardFragmentTestActivity.kt
Show resolved
Hide resolved
app/src/main/java/org/oppia/app/topic/conceptcard/testing/ConceptCardFragmentTestActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/res/layout/concept_card_fragment_test_activity.xml
Outdated
Show resolved
Hide resolved
app/src/main/res/layout/concept_card_fragment_test_activity.xml
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/topic/conceptcard/ConceptCardFragmentTest.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/app/topic/conceptcard/ConceptCardFragmentTest.kt
Show resolved
Hide resolved
@BenHenning PTAL. Add test cases for ConceptCard using ActivityTestRule (for screen rotation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,551 @@ | |||
<vector xmlns:android="http://schemas.android.com/apk/res/android" android:width="305dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't these already imported per https://github.com/oppia/oppia-android/blob/develop/app/src/main/res/drawable/lesson_thumbnail_graphic_baker.xml?
Conflicts: app/build.gradle app/src/main/AndroidManifest.xml app/src/main/java/org/oppia/app/activity/ActivityComponent.kt app/src/main/java/org/oppia/app/fragment/FragmentComponent.kt app/src/main/java/org/oppia/app/player/audio/AudioFragment.kt
Conflicts: app/src/main/java/org/oppia/app/activity/ActivityComponent.kt
…view-adapter' into concept-card-topic-controller
@BenHenning PTAL. I merged in the bindable recyclerview to this branch again, so only my changes show in the diff. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jamesxu0! I only had one concern around the configuration testing, but otherwise this LGTM!
@Test | ||
fun testConceptCardFragment_openDialogFragmentWithSkill2_afterConfigurationChange_workedExamplesAreDisplayed() { | ||
onView(withId(R.id.open_dialog_2)).perform(click()) | ||
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work as-is or do you also need to call recreate on the activity scenario? Did you verify via step debugging that the activity is recreated? Please update test accordingly to make sure that it does go through the rotation as expected.
Explanation
Fixes #160: Integrates the topic controller into Concept Card. Concept Card uses data binding to bind the explanation and worked examples into text views (blocked on #26). Uses the binable recyclerview introduced in PR #172, which has not been merged yet.
Checklist