Skip to content

Commit

Permalink
Fixes #160: Add HTML parsing to ConceptCardFragment (#331)
Browse files Browse the repository at this point in the history
* Added html parsing

* Added title to toolbar

* refactored viewModel to use fragment

* Moved sets to ViewModel

* updated name

* Consolidated recyclerview and wrote text cases that check for rich text

* Moved test activity to test folder

* removed long file

* Removed recyclerview and worked examples

* made host activity dismiss fragment

* Added comment for dismiss

* Fixed alignment

* Fixed formatting

* changed to getOrDefault

* changed test names
  • Loading branch information
jamesxu0 authored Nov 19, 2019
1 parent 6c0b771 commit b60a8e1
Showing 20 changed files with 185 additions and 137 deletions.
2 changes: 1 addition & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -64,7 +64,7 @@
<activity
android:name=".testing.TopicTestActivityForStory"
android:theme="@style/OppiaThemeWithoutActionBar" />
<activity android:name=".topic.conceptcard.testing.ConceptCardFragmentTestActivity" />
<activity android:name=".testing.ConceptCardFragmentTestActivity" />
<activity
android:name=".topic.questionplayer.QuestionPlayerActivity"
android:screenOrientation="portrait" />
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ import org.oppia.app.testing.HtmlParserTestActivity
import org.oppia.app.testing.TopicTestActivity
import org.oppia.app.testing.TopicTestActivityForStory
import org.oppia.app.topic.TopicActivity
import org.oppia.app.topic.conceptcard.testing.ConceptCardFragmentTestActivity
import org.oppia.app.testing.ConceptCardFragmentTestActivity
import org.oppia.app.topic.questionplayer.QuestionPlayerActivity
import javax.inject.Provider

Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package org.oppia.app.topic.conceptcard.testing
package org.oppia.app.testing

import android.os.Bundle
import org.oppia.app.activity.InjectableAppCompatActivity
import org.oppia.app.topic.conceptcard.ConceptCardFragment
import org.oppia.app.topic.conceptcard.ConceptCardListener
import javax.inject.Inject

/** Test Activity used for testing ConceptCardFragment */
class ConceptCardFragmentTestActivity : InjectableAppCompatActivity() {
class ConceptCardFragmentTestActivity : InjectableAppCompatActivity(), ConceptCardListener {

@Inject lateinit var conceptCardFragmentTestActivityController: ConceptCardFragmentTestActivityPresenter

@@ -14,4 +16,16 @@ class ConceptCardFragmentTestActivity : InjectableAppCompatActivity() {
activityComponent.inject(this)
conceptCardFragmentTestActivityController.handleOnCreate()
}

override fun dismiss() {
getConceptCardFragment()?.dismiss()
}

private fun getConceptCardFragment(): ConceptCardFragment? {
return supportFragmentManager.findFragmentByTag(TAG_CONCEPT_CARD_DIALOG) as ConceptCardFragment?
}

companion object {
internal const val TAG_CONCEPT_CARD_DIALOG = "CONCEPT_CARD_DIALOG"
}
}
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
package org.oppia.app.topic.conceptcard.testing
package org.oppia.app.testing

import androidx.appcompat.app.AppCompatActivity
import kotlinx.android.synthetic.main.concept_card_fragment_test_activity.*
import org.oppia.app.R
import org.oppia.app.testing.ConceptCardFragmentTestActivity.Companion.TAG_CONCEPT_CARD_DIALOG
import org.oppia.app.topic.conceptcard.ConceptCardFragment
import org.oppia.domain.topic.TEST_SKILL_ID_0
import org.oppia.domain.topic.TEST_SKILL_ID_1
import org.oppia.domain.topic.TEST_SKILL_ID_2
import javax.inject.Inject

private const val TAG_CONCEPT_CARD_DIALOG = "CONCEPT_CARD_DIALOG"

/** The presenter for [ConceptCardFragmentTestActivity] */
class ConceptCardFragmentTestActivityPresenter @Inject constructor(private val activity: AppCompatActivity) {
fun handleOnCreate() {
activity.setContentView(R.layout.concept_card_fragment_test_activity)
activity.open_dialog_1.setOnClickListener {
val frag = ConceptCardFragment.newInstance(TEST_SKILL_ID_1)
activity.open_dialog_0.setOnClickListener {
val frag = ConceptCardFragment.newInstance(TEST_SKILL_ID_0)
frag.showNow(activity.supportFragmentManager, TAG_CONCEPT_CARD_DIALOG)
}
activity.open_dialog_2.setOnClickListener {
val frag = ConceptCardFragment.newInstance(TEST_SKILL_ID_2)
activity.open_dialog_1.setOnClickListener {
val frag = ConceptCardFragment.newInstance(TEST_SKILL_ID_1)
frag.showNow(activity.supportFragmentManager, TAG_CONCEPT_CARD_DIALOG)
}
}
7 changes: 6 additions & 1 deletion app/src/main/java/org/oppia/app/testing/TopicTestActivity.kt
Original file line number Diff line number Diff line change
@@ -14,13 +14,14 @@ import org.oppia.app.topic.TopicActivityPresenter
import org.oppia.app.topic.TopicFragment
import org.oppia.app.topic.TopicTab
import org.oppia.app.topic.conceptcard.ConceptCardFragment
import org.oppia.app.topic.conceptcard.ConceptCardListener
import org.oppia.app.topic.questionplayer.QuestionPlayerActivity
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import javax.inject.Inject

/** The activity for testing [TopicFragment]. */
class TopicTestActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerListener, RouteToConceptCardListener,
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener {
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener, ConceptCardListener {
@Inject
lateinit var topicActivityPresenter: TopicActivityPresenter

@@ -50,6 +51,10 @@ class TopicTestActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerLi
}
}

override fun dismiss() {
getConceptCardFragment()?.dismiss()
}

override fun routeToExploration(explorationId: String) {
startActivity(ExplorationActivity.createExplorationActivityIntent(this, explorationId))
}
Original file line number Diff line number Diff line change
@@ -14,15 +14,16 @@ import org.oppia.app.topic.TopicActivityPresenter
import org.oppia.app.topic.TopicFragment
import org.oppia.app.topic.TopicTab
import org.oppia.app.topic.conceptcard.ConceptCardFragment
import org.oppia.app.topic.conceptcard.ConceptCardListener
import org.oppia.app.topic.questionplayer.QuestionPlayerActivity
import org.oppia.domain.topic.TEST_STORY_ID_1
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import javax.inject.Inject

/** The test activity for [TopicFragment] to test displaying story by storyId. */
class TopicTestActivityForStory : InjectableAppCompatActivity(), RouteToQuestionPlayerListener,
RouteToConceptCardListener,
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener {
RouteToConceptCardListener, RouteToTopicPlayListener, RouteToStoryListener,
RouteToExplorationListener, ConceptCardListener {
@Inject
lateinit var topicActivityPresenter: TopicActivityPresenter

@@ -52,6 +53,10 @@ class TopicTestActivityForStory : InjectableAppCompatActivity(), RouteToQuestion
}
}

override fun dismiss() {
getConceptCardFragment()?.dismiss()
}

override fun routeToExploration(explorationId: String) {
startActivity(ExplorationActivity.createExplorationActivityIntent(this, explorationId))
}
7 changes: 6 additions & 1 deletion app/src/main/java/org/oppia/app/topic/TopicActivity.kt
Original file line number Diff line number Diff line change
@@ -8,14 +8,15 @@ import org.oppia.app.home.RouteToExplorationListener
import org.oppia.app.player.exploration.ExplorationActivity
import org.oppia.app.story.StoryActivity
import org.oppia.app.topic.conceptcard.ConceptCardFragment
import org.oppia.app.topic.conceptcard.ConceptCardListener
import org.oppia.app.topic.questionplayer.QuestionPlayerActivity
import javax.inject.Inject

const val TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY = "TopicActivity.topic_id"

/** The activity for displaying [TopicFragment]. */
class TopicActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerListener, RouteToConceptCardListener,
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener {
RouteToTopicPlayListener, RouteToStoryListener, RouteToExplorationListener, ConceptCardListener {
private lateinit var topicId: String
private lateinit var storyId: String
@Inject
@@ -52,6 +53,10 @@ class TopicActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerListen
}
}

override fun dismiss() {
getConceptCardFragment()?.dismiss()
}

override fun routeToExploration(explorationId: String) {
startActivity(ExplorationActivity.createExplorationActivityIntent(this, explorationId))
}
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ class ConceptCardFragment : InjectableDialogFragment() {
}
}

@Inject lateinit var conceptCardPresenter: ConceptCardPresenter
@Inject lateinit var conceptCardFragmentPresenter: ConceptCardFragmentPresenter

override fun onAttach(context: Context) {
super.onAttach(context)
@@ -45,7 +45,7 @@ class ConceptCardFragment : InjectableDialogFragment() {
super.onCreateView(inflater, container, savedInstanceState)
val args = checkNotNull(arguments) { "Expected arguments to be passed to ConceptCardFragment" }
val skillId = checkNotNull(args.getString(KEY_SKILL_ID)) { "Expected skillId to be passed to ConceptCardFragment" }
return conceptCardPresenter.handleCreateView(inflater, container, skillId)
return conceptCardFragmentPresenter.handleCreateView(inflater, container, skillId)
}

override fun onStart() {
Original file line number Diff line number Diff line change
@@ -3,35 +3,35 @@ package org.oppia.app.topic.conceptcard
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.Fragment
import org.oppia.app.R
import org.oppia.app.databinding.ConceptCardExampleViewBinding
import org.oppia.app.databinding.ConceptCardFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.SubtitledHtml
import org.oppia.app.recyclerview.BindableAdapter
import org.oppia.app.viewmodel.ViewModelProvider
import javax.inject.Inject

/** Presenter for [ConceptCardFragment], sets up bindings from ViewModel */
@FragmentScope
class ConceptCardPresenter @Inject constructor(
class ConceptCardFragmentPresenter @Inject constructor(
private val fragment: Fragment,
private val viewModelProvider: ViewModelProvider<ConceptCardViewModel>
){
) {
private lateinit var skillId: String

/** Sets up data binding and adapter for RecyclerView */
fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?, skillId: String): View? {
val viewModel = getConceptCardViewModel()
viewModel.setSkillId(skillId)
/**
* Sets up data binding and toolbar.
* Host activity must inherit ConceptCardListener to dismiss this fragment.
*/
fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?, id: String): View? {
val binding = ConceptCardFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
val viewModel = getConceptCardViewModel()

skillId = id
viewModel.setSkillIdAndBinding(skillId, binding)

binding.conceptCardToolbar.setNavigationIcon(R.drawable.ic_close_white_24dp)
binding.conceptCardToolbar.setNavigationOnClickListener {
(fragment as? DialogFragment)?.dismiss()
}
binding.workedExamples.apply {
adapter = createRecyclerViewAdapter()
(fragment.requireActivity() as? ConceptCardListener)?.dismiss()
}

binding.let {
@@ -44,13 +44,4 @@ class ConceptCardPresenter @Inject constructor(
private fun getConceptCardViewModel(): ConceptCardViewModel {
return viewModelProvider.getForFragment(fragment, ConceptCardViewModel::class.java)
}

private fun createRecyclerViewAdapter(): BindableAdapter<SubtitledHtml> {
return BindableAdapter.Builder
.newBuilder<SubtitledHtml>()
.registerViewDataBinderWithSameModelType(
inflateDataBinding = ConceptCardExampleViewBinding::inflate,
setViewModel = ConceptCardExampleViewBinding::setSubtitledHtml)
.build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.oppia.app.topic.conceptcard

/** Allows parent activity to dismiss the [ConceptCardFragment] */
interface ConceptCardListener {
/** Called when the concept card dialog should be dismissed. */
fun dismiss()
}
Original file line number Diff line number Diff line change
@@ -1,38 +1,42 @@
package org.oppia.app.topic.conceptcard

import android.widget.TextView
import androidx.lifecycle.LiveData
import androidx.lifecycle.Transformations
import androidx.lifecycle.ViewModel
import org.oppia.app.databinding.ConceptCardFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.ConceptCard
import org.oppia.app.model.SubtitledHtml
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
import org.oppia.util.parser.ConceptCardHtmlParserEntityType
import org.oppia.util.parser.HtmlParser
import javax.inject.Inject

/** [ViewModel] for concept card, providing rich text and worked examples */
@FragmentScope
class ConceptCardViewModel @Inject constructor(
private val topicController: TopicController,
private val logger: Logger
private val logger: Logger,
private val htmlParserFactory: HtmlParser.Factory,
@ConceptCardHtmlParserEntityType private val entityType: String
) : ViewModel() {

private lateinit var skillId: String
private lateinit var binding: ConceptCardFragmentBinding

/** Live Data for concept card explanation */
val conceptCardLiveData: LiveData<ConceptCard> by lazy {
processConceptCardLiveData()
}

/** Live Data for concept card worked examples. */
val workedExamplesLiveData: LiveData<List<SubtitledHtml>> by lazy {
processWorkedExamplesLiveData()
val explanationLiveData: LiveData<CharSequence> by lazy {
processExplanationLiveData()
}

/** Sets the value of skillId. Must be called before setting ViewModel to binding. */
fun setSkillId(id: String) {
/** Sets the value of skillId and binding. Must be called before setting ViewModel to binding */
fun setSkillIdAndBinding(id: String, binding: ConceptCardFragmentBinding) {
skillId = id
this.binding = binding
}

private val conceptCardResultLiveData: LiveData<AsyncResult<ConceptCard>> by lazy {
@@ -43,21 +47,23 @@ class ConceptCardViewModel @Inject constructor(
return Transformations.map(conceptCardResultLiveData, ::processConceptCardResult)
}

private fun processWorkedExamplesLiveData(): LiveData<List<SubtitledHtml>> {
return Transformations.map(conceptCardResultLiveData, ::processConceptCardWorkExamples)
private fun processExplanationLiveData(): LiveData<CharSequence> {
return Transformations.map(conceptCardResultLiveData, ::processExplanationResult)
}

private fun processConceptCardResult(conceptCardResult: AsyncResult<ConceptCard>): ConceptCard {
if (conceptCardResult.isFailure()) {
logger.e("ConceptCardFragment", "Failed to retrieve Concept Card: " + conceptCardResult.getErrorOrNull())
logger.e("ConceptCardFragment", "Failed to retrieve Concept Card", conceptCardResult.getErrorOrNull()!!)
}
return conceptCardResult.getOrDefault(ConceptCard.getDefaultInstance())
}

private fun processConceptCardWorkExamples(conceptCardResult: AsyncResult<ConceptCard>): List<SubtitledHtml> {
private fun processExplanationResult(conceptCardResult: AsyncResult<ConceptCard>): CharSequence {
if (conceptCardResult.isFailure()) {
logger.e("ConceptCardFragment", "Failed to retrieve Concept Card: " + conceptCardResult.getErrorOrNull())
logger.e("ConceptCardFragment", "Failed to retrieve Concept Card", conceptCardResult.getErrorOrNull()!!)
}
return conceptCardResult.getOrDefault(ConceptCard.getDefaultInstance()).workedExampleList
val conceptCard = conceptCardResult.getOrDefault(ConceptCard.getDefaultInstance())
return htmlParserFactory.create(entityType, skillId)
.parseOppiaHtml(conceptCard.explanation.html, binding.conceptCardExplanationText)
}
}
25 changes: 0 additions & 25 deletions app/src/main/res/layout/concept_card_example_view.xml

This file was deleted.

Loading

0 comments on commit b60a8e1

Please sign in to comment.