Skip to content
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

Fix #359: Add descriptions for fractions & ratios topics #394

Merged
merged 4 commits into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions app/src/main/java/org/oppia/app/topic/TopicActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ const val TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY = "TopicActivity.topic_id"
class TopicActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerListener, RouteToConceptCardListener,
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener {
private lateinit var topicId: String
private lateinit var storyId: String
private var storyId: String? = null
@Inject
lateinit var topicActivityPresenter: TopicActivityPresenter

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
activityComponent.inject(this)
topicId = checkNotNull(intent?.getStringExtra(org.oppia.app.topic.TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY) ?: "")
{
topicId = checkNotNull(intent?.getStringExtra(org.oppia.app.topic.TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY)) {
"Expected topic ID to be included in intent for TopicActivity."
}
storyId = intent?.getStringExtra(TOPIC_ACTIVITY_STORY_ID_ARGUMENT_KEY) ?: ""
storyId = intent?.getStringExtra(TOPIC_ACTIVITY_STORY_ID_ARGUMENT_KEY)
topicActivityPresenter.handleOnCreate(topicId, storyId)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ const val STORY_ID_ARGUMENT_KEY = "story_id"
@ActivityScope
class TopicActivityPresenter @Inject constructor(private val activity: AppCompatActivity) {

fun handleOnCreate(topicId: String, storyId: String) {
fun handleOnCreate(topicId: String, storyId: String?) {
activity.setContentView(R.layout.topic_activity)
if (getTopicFragment() == null) {
val topicFragment = TopicFragment()
val args = Bundle()
args.putString(TOPIC_ID_ARGUMENT_KEY, topicId)
if (topicId.isNotEmpty() && storyId.isNotEmpty())
if (storyId != null) {
args.putString(STORY_ID_ARGUMENT_KEY, storyId)
}
topicFragment.arguments = args
activity.supportFragmentManager.beginTransaction().add(
R.id.topic_fragment_placeholder,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ class TopicFragmentPresenter @Inject constructor(
viewPager.setCurrentItem(tab.ordinal, true)
}

private fun setUpViewPager(viewPager: ViewPager, topicId: String?) {
val adapter = ViewPagerAdapter(fragment.fragmentManager!!, topicId!!, storyId)
private fun setUpViewPager(viewPager: ViewPager, topicId: String) {
val adapter = ViewPagerAdapter(fragment.childFragmentManager, topicId, storyId)
viewPager.adapter = adapter
tabLayout.setupWithViewPager(viewPager)
tabLayout.getTabAt(0)!!.setText(fragment.getString(R.string.overview)).setIcon(tabIcons[0])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import androidx.fragment.app.Fragment
import androidx.lifecycle.LiveData
import androidx.lifecycle.Observer
import androidx.lifecycle.Transformations
import org.oppia.app.R
import org.oppia.app.databinding.TopicOverviewFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.Topic
Expand All @@ -17,6 +18,7 @@ import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
import org.oppia.util.parser.HtmlParser
import javax.inject.Inject

/** The presenter for [TopicOverviewFragment]. */
Expand All @@ -26,11 +28,15 @@ class TopicOverviewFragmentPresenter @Inject constructor(
private val fragment: Fragment,
private val viewModelProvider: ViewModelProvider<TopicOverviewViewModel>,
private val logger: Logger,
private val topicController: TopicController
private val topicController: TopicController,
private val htmlParserFactory: HtmlParser.Factory
) {
private val routeToTopicPlayListener = activity as RouteToTopicPlayListener
private val topicOverviewViewModel = getTopicOverviewViewModel()
private lateinit var topicId: String
private val htmlParser: HtmlParser by lazy {
htmlParserFactory.create(/* entityType= */ "topic", topicId)
}

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
topicId = checkNotNull(fragment.arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
Expand All @@ -46,7 +52,7 @@ class TopicOverviewFragmentPresenter @Inject constructor(
return binding.root
}

fun seeMoreClicked(v: View) {
fun seeMoreClicked(@Suppress("UNUSED_PARAMETER") v: View) {
routeToTopicPlayListener.routeToTopicPlayFragment()
}

Expand All @@ -57,8 +63,14 @@ class TopicOverviewFragmentPresenter @Inject constructor(
private val topicLiveData: LiveData<Topic> by lazy { getTopicList() }

private fun subscribeToTopicLiveData() {
topicLiveData.observe(fragment, Observer<Topic> { result ->
topicOverviewViewModel.topic.set(result)
topicLiveData.observe(fragment, Observer<Topic> { topic ->
topicOverviewViewModel.topic.set(topic)
topicOverviewViewModel.topicDescription.set(
htmlParser.parseOppiaHtml(
topic.description,
fragment.requireView().findViewById(R.id.topic_description_text_view)
)
)
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ import javax.inject.Inject
class TopicOverviewViewModel @Inject constructor() : ObservableViewModel() {
val topic = ObservableField<Topic>(Topic.getDefaultInstance())

val topicDescription = ObservableField<CharSequence>("")

var downloadStatusIndicatorDrawableResourceId = ObservableField<Int>(R.drawable.ic_available_offline_primary_24dp)
}
2 changes: 1 addition & 1 deletion app/src/main/res/layout/topic_overview_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
android:layout_marginTop="16dp"
android:layout_marginEnd="24dp"
android:gravity="start"
android:text="@{viewModel.topic.description}"
android:text="@{viewModel.topicDescription}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="16sp"
app:layout_constraintEnd_toEndOf="parent"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,116 +1,92 @@
package org.oppia.app.topic.overview

import android.app.Application
import android.content.Context
import android.content.pm.ActivityInfo
import android.text.SpannedString
import android.text.style.StyleSpan
import android.widget.TextView
import androidx.test.core.app.ActivityScenario
import androidx.test.core.app.ApplicationProvider
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isClickable
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.rule.ActivityTestRule
import dagger.BindsInstance
import dagger.Component
import dagger.Module
import dagger.Provides
import kotlinx.coroutines.CoroutineDispatcher
import com.google.common.truth.Truth.assertThat
import org.hamcrest.Matchers.containsString
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.oppia.app.R
import org.oppia.app.topic.TopicActivity
import org.oppia.app.utility.EspressoTestsMatchers.withDrawable
import org.oppia.util.threading.BackgroundDispatcher
import org.oppia.util.threading.BlockingDispatcher
import javax.inject.Singleton

private const val TEST_TOPIC_ID = "GJ2rLXRKD5hw"
private const val TOPIC_NAME = "Fractions"
private const val TOPIC_DESCRIPTION =
"You'll often need to talk about part of an object or group. For example, a jar of milk might be half-full, or " +
"some of the eggs in a box might have broken. In these lessons, you'll learn to use fractions to describe " +
"situations like these."

// NOTE TO DEVELOPERS: this test must be annotated with @LooperMode(LooperMode.MODE.PAUSED) to pass.
/** Tests for [TopicOverviewFragment]. */
@RunWith(AndroidJUnit4::class)
class TopicOverviewFragmentTest {

private val topicThumbnail = R.drawable.lesson_thumbnail_graphic_child_with_cupcakes

private val topicName = "Second Test Topic"

private val topicDescription =
"A topic considering the various implications of having especially long topic descriptions. " +
"These descriptions almost certainly need to wrap, which should be interesting in the UI (especially on " +
"small screens). Consider also that there may even be multiple points pertaining to a topic, some of which " +
"may require expanding the description section in order to read the whole topic description."

@get:Rule
var activityTestRule: ActivityTestRule<TopicActivity> = ActivityTestRule(
TopicActivity::class.java, /* initialTouchMode= */ true, /* launchActivity= */ false
)
private val topicThumbnail = R.drawable.lesson_thumbnail_graphic_child_with_fractions_homework

@Test
fun testTopicOverviewFragment_loadFragment_checkTopicName_isCorrect() {
ActivityScenario.launch(TopicActivity::class.java).use {
onView(withId(R.id.topic_name_text_view)).check(matches(withText(containsString(topicName))))
launchTopicActivityIntent(TEST_TOPIC_ID).use {
onView(withId(R.id.topic_name_text_view)).check(matches(withText(containsString(TOPIC_NAME))))
}
}

// TODO(#135): Update this test case to check on click of See More play tab is shown.
@Test
fun testTopicOverviewFragment_loadFragment_seeMoreIsClickable() {
ActivityScenario.launch(TopicActivity::class.java).use {
launchTopicActivityIntent(TEST_TOPIC_ID).use {
onView(withId(R.id.see_more_text_view)).check(matches(isClickable()))
}
}

@Test
fun testTopicOverviewFragment_loadFragmentWithTestTopicId1_checkTopicDescription_isCorrect() {
ActivityScenario.launch(TopicActivity::class.java).use {
onView(withId(R.id.topic_description_text_view)).check(matches(withText(containsString(topicDescription))))
launchTopicActivityIntent(TEST_TOPIC_ID).use {
onView(withId(R.id.topic_description_text_view)).check(matches(withText(containsString(TOPIC_DESCRIPTION))))
}
}

@Test
fun testTopicOverviewFragment_loadFragmentWithTestTopicId1_checkTopicDescription_hasRichText() {
launchTopicActivityIntent(TEST_TOPIC_ID).use { scenario ->
scenario.onActivity { activity ->
val descriptionTextView: TextView = activity.findViewById(R.id.topic_description_text_view)
val descriptionText = descriptionTextView.text as SpannedString
val spans = descriptionText.getSpans(0, descriptionText.length, StyleSpan::class.java)
assertThat(spans).isNotEmpty()
}
}
}

@Test
fun testTopicOverviewFragment_loadFragment_configurationChange_checkTopicThumbnail_isCorrect() {
ActivityScenario.launch(TopicActivity::class.java).use {
launchTopicActivityIntent(TEST_TOPIC_ID).use {
onView(withId(R.id.topic_thumbnail_image_view)).check(matches(withDrawable(topicThumbnail)))
}
}

@Test
fun testTopicOverviewFragment_loadFragment_configurationChange_checkTopicName_isCorrect() {
activityTestRule.launchActivity(null)
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
onView(withId(R.id.topic_name_text_view)).check(matches(withText(containsString(topicName))))
}
launchTopicActivityIntent(TEST_TOPIC_ID).use { scenario ->
scenario.onActivity { activity ->
activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
}

@Module
class TestModule {
@Provides
@Singleton
fun provideContext(application: Application): Context {
return application
}

// TODO(#89): Introduce a proper IdlingResource for background dispatchers to ensure they all complete before
// proceeding in an Espresso test. This solution should also be interoperative with Robolectric contexts by using a
// test coroutine dispatcher.

@Singleton
@Provides
@BackgroundDispatcher
fun provideBackgroundDispatcher(@BlockingDispatcher blockingDispatcher: CoroutineDispatcher): CoroutineDispatcher {
return blockingDispatcher
onView(withId(R.id.topic_name_text_view)).check(matches(withText(containsString(TOPIC_NAME))))
}
}

@Singleton
@Component(modules = [TestModule::class])
interface TestApplicationComponent {
@Component.Builder
interface Builder {
@BindsInstance
fun setApplication(application: Application): Builder

fun build(): TestApplicationComponent
}
private fun launchTopicActivityIntent(topicId: String): ActivityScenario<TopicActivity> {
val intent = TopicActivity.createTopicActivityIntent(ApplicationProvider.getApplicationContext(), topicId)
return ActivityScenario.launch(intent)
}
}
2 changes: 1 addition & 1 deletion domain/src/main/assets/fractions_topic.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"topic": {
"story_reference_schema_version": 1,
"description": "",
"description": "You'll often need to talk about <b>part</b> of an object or group. For example, a jar of milk might be half-full, or some of the eggs in a box might have broken. In these lessons, you'll learn to use <b>fractions</b> to describe situations like these.",
"canonical_story_references": [
{
"story_id": "wANbh4oOClga",
Expand Down
2 changes: 1 addition & 1 deletion domain/src/main/assets/ratios_topic.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"topic": {
"story_reference_schema_version": 1,
"description": "",
"description": "Many everyday problems involve thinking about proportions. For example, if 30 apples cost 6 dollars, how much would 12 apples cost? In these lessons, you'll learn about how to use ratios and proportional reasoning to solve problems like this.",
"canonical_story_references": [],
"language_code": "en",
"id": "omzF4oqgeTXd",
Expand Down