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 #161: Exploration player contentcard supports rich-text part -2 #229

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.oppia.app.home.HomeFragment
import org.oppia.app.player.exploration.ExplorationFragment
import org.oppia.app.player.state.StateFragment
import org.oppia.app.player.audio.AudioFragment
import org.oppia.app.player.content.ContentListFragment
import org.oppia.app.testing.BindableAdapterTestFragment
import org.oppia.app.topic.TopicFragment
import org.oppia.app.topic.conceptcard.ConceptCardFragment
Expand All @@ -28,6 +29,7 @@ interface FragmentComponent {

fun inject(audioFragment: AudioFragment)
fun inject(conceptCardFragment: ConceptCardFragment)
fun inject(contentListFragment: ContentListFragment)
fun inject(explorationFragment: ExplorationFragment)
fun inject(homeFragment: HomeFragment)
fun inject(questionPlayerFragment: QuestionPlayerFragment)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package org.oppia.app.player.content

import android.content.Context
import android.text.Spannable
import android.view.LayoutInflater
import android.view.ViewGroup
import androidx.databinding.DataBindingUtil
import androidx.databinding.ViewDataBinding
import androidx.databinding.library.baseAdapters.BR
import androidx.recyclerview.widget.RecyclerView
import kotlinx.android.synthetic.main.content_card_items.view.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need any synthetic imports. Please remove.

import kotlinx.android.synthetic.main.interation_feedback_card_item.view.*
import org.oppia.app.R
import org.oppia.app.databinding.ContentCardItemsBinding
import org.oppia.app.databinding.InterationFeedbackCardItemBinding
import org.oppia.util.data.HtmlParser

// TODO(#172): Make use of generic data-binding-enabled RecyclerView adapter
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
/** Adapter to bind the contents to the [RecyclerView]. It handles rich-text content. */
class ContentCardAdapter(
private val context: Context,
private val entity_type: String,
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
private val entity_id: String,
val contentList: MutableList<ContentViewModel>
) : RecyclerView.Adapter<RecyclerView.ViewHolder>() {

val VIEW_TYPE_CONTENT = 1
val VIEW_TYPE_INTERACTION_FEEDBACK = 2

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
return when (viewType) {
VIEW_TYPE_CONTENT -> {
val inflater = LayoutInflater.from(parent.getContext())
val binding =
DataBindingUtil.inflate<ContentCardItemsBinding>(inflater, R.layout.content_card_items, parent, false)
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
ContentViewHolder(binding)
}
VIEW_TYPE_INTERACTION_FEEDBACK -> {
val inflater = LayoutInflater.from(parent.getContext())
val binding =
DataBindingUtil.inflate<InterationFeedbackCardItemBinding>(
inflater,
R.layout.interation_feedback_card_item,
parent,
false
)
InteractionFeedbackViewHolder(binding)
}
else -> throw IllegalArgumentException("Invalid view type")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest including the viewType parameter in this exception cause since it'll help provide context if the exception is ever thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand what viewType parameter you mean. Can you please elaborate on this.
We have only two viewTypes over here one is content and other is learner's interaction.

}
}

override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
when (holder.itemViewType) {
VIEW_TYPE_CONTENT -> (holder as ContentViewHolder).bind(contentList!!.get(position).htmlContent.get())
VIEW_TYPE_INTERACTION_FEEDBACK -> (holder as InteractionFeedbackViewHolder).bind(contentList!!.get(position).htmlContent.get())
}
}

// Determines the appropriate ViewType according to the content_id.
override fun getItemViewType(position: Int): Int {
return if (!contentList!!.get(position).contentId.get()!!.contains("content") &&
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
!contentList!!.get(position).contentId.get()!!.contains(
"Feedback"
)
) {
VIEW_TYPE_INTERACTION_FEEDBACK
} else {
VIEW_TYPE_CONTENT
}
}

override fun getItemCount(): Int {
return contentList!!.size
}

private inner class ContentViewHolder(val binding: ViewDataBinding) : RecyclerView.ViewHolder(binding.root) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just make these regular nested classes instead of inner classes? Prefer not exposing the outer class instance to these inner classes if we don't need to.

internal fun bind(rawString: String?) {
binding.setVariable(BR.htmlContent, rawString)
binding.executePendingBindings();
val html: Spannable = HtmlParser(context, entity_type, entity_id).parseHtml(rawString, binding.root.tv_contents)
binding.root.tv_contents.text = html
}
}

private inner class InteractionFeedbackViewHolder(val binding: ViewDataBinding) :
RecyclerView.ViewHolder(binding.root) {
internal fun bind(rawString: String?) {
binding.setVariable(BR.htmlContent, rawString)
binding.executePendingBindings();
val html: Spannable =
HtmlParser(context, entity_type, entity_id).parseHtml(rawString, binding.root.tv_interaction_feedback)
binding.root.tv_interaction_feedback.text = html
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.oppia.app.player.content

import android.content.Context
import android.os.Bundle
import android.util.Log
import androidx.fragment.app.Fragment
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Button

import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import org.oppia.app.R
import org.oppia.app.fragment.InjectableFragment
import org.oppia.data.backends.gae.NetworkModule
import org.oppia.data.backends.gae.model.GaeExplorationContainer
import org.oppia.data.backends.gae.model.GaeState
import org.oppia.data.backends.gae.model.GaeSubtitledHtml
import retrofit2.Call
import retrofit2.Callback
import retrofit2.Response
import java.lang.Exception

import java.util.ArrayList
import javax.inject.Inject

/** Fragment that displays contents that supports rich-text. */
class ContentListFragment : InjectableFragment() {

@Inject
lateinit var contentListFragmentPresenter: ContentListFragmentPresenter

override fun onAttach(context: Context?) {
super.onAttach(context)
fragmentComponent.inject(this)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
return contentListFragmentPresenter.handleCreateView(inflater, container)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package org.oppia.app.player.content

import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.Fragment
import androidx.recyclerview.widget.LinearLayoutManager
import org.oppia.app.databinding.ContentListFragmentBinding
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.data.backends.gae.model.GaeSubtitledHtml
import org.oppia.domain.exploration.ExplorationProgressController
import org.oppia.util.logging.Logger
import javax.inject.Inject

/** Presenter for [ContentListFragment]. */
class ContentListFragmentPresenter @Inject constructor(
private val fragment: Fragment,
private val viewModelProvider: ViewModelProvider<ContentViewModel>,
private val logger: Logger
) {

private val entity_type: String = "exploration"
veena14cs marked this conversation as resolved.
Show resolved Hide resolved

lateinit var contentCardAdapter: ContentCardAdapter

var contentList: MutableList<ContentViewModel> = ArrayList()

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
val binding = ContentListFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
binding.let {
it.viewModel = getContentViewModel()
it.contentFragment = fragment as ContentListFragment
it.lifecycleOwner = fragment
}
binding.recyclerview.apply {
binding.recyclerview.layoutManager = LinearLayoutManager(context)
contentCardAdapter =
ContentCardAdapter(context, entity_type, fragment.arguments!!.getString("exploration_id"), contentList);
binding.recyclerview.adapter = contentCardAdapter
}
getContentList()

return binding.root
}

private fun getContentList() {
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
getContentViewModel().setContentId(fragment.arguments!!.getString("content_id"))
getContentViewModel().setHtmlContent(fragment.arguments!!.getString("htmlContent"))
logger.d("ContentListFragment", "htmlcontent: ${fragment.arguments!!.getString("htmlContent")}")
contentList.add(getContentViewModel())
contentCardAdapter!!.notifyDataSetChanged()
}

private fun getContentViewModel(): ContentViewModel {
return viewModelProvider.getForFragment(fragment, ContentViewModel::class.java)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.oppia.app.player.content

import androidx.databinding.ObservableField
import androidx.lifecycle.ViewModel
import org.oppia.app.fragment.FragmentScope
import javax.inject.Inject

/** [ViewModel] for content-card state. */
@FragmentScope
class ContentViewModel @Inject constructor() : ViewModel() {

val contentId = ObservableField<String>("content_id")
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
val htmlContent = ObservableField<String>("html")

fun setContentId(content_id: String) {
contentId.set(content_id)
}

fun setHtmlContent(html: String) {
htmlContent.set(html)
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
package org.oppia.app.player.state

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
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.StateFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.CellularDataPreference
import org.oppia.app.model.EphemeralState
import org.oppia.app.player.audio.CellularDataDialogFragment
import org.oppia.app.player.content.ContentListFragment
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.audio.CellularDialogController
import org.oppia.domain.exploration.ExplorationProgressController
Expand All @@ -33,6 +36,7 @@ class StateFragmentPresenter @Inject constructor(

private var showCellularDataDialog = true
private var useCellularData = false
private val dummyExploration_id: String = "umPkwp0L1M0-"

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
cellularDialogController.getCellularDataPreference()
Expand Down Expand Up @@ -95,9 +99,29 @@ class StateFragmentPresenter @Inject constructor(
private fun subscribeToCurrentState() {
ephemeralStateLiveData.observe(fragment, Observer<EphemeralState> { result ->
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
logger.d("StateFragment", "getCurrentState: ${result.state.name}")
if (getContentListFragment() == null) {
val contentListFragment = ContentListFragment()
val args = Bundle()
args.putString("exploration_id", dummyExploration_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be using the same exploration ID that's being passed into StateFragmentPresenter?

If the explorations that are being included in the APK itself are not satisfactory for testing the state fragment, feel free to modify them to add additional rich text contents as needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Shouldn't we be using the same exploration ID that's being passed into StateFragmentPresenter?

Yes we could use. I added dummy exploration id for testing image and modified json.

if (result.state.content.contentId.equals("")) {
args.putString("content_id", "content")
} else {
args.putString("content_id", result.state.content.contentId)
}
args.putString("htmlContent", result.state.content.html)
contentListFragment.arguments = args
fragment.childFragmentManager.beginTransaction().add(
R.id.content_list_fragment_placeholder,
contentListFragment
).commitNow()
}
})
}

private fun getContentListFragment(): ContentListFragment? {
return fragment.childFragmentManager.findFragmentById(R.id.content_list_fragment_placeholder) as ContentListFragment?
}

private val ephemeralStateLiveData: LiveData<EphemeralState> by lazy {
getEphemeralState()
}
Expand Down
22 changes: 22 additions & 0 deletions app/src/main/res/layout/content_card_items.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<layout xmlns:android="http://schemas.android.com/apk/res/android">
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
<data>
<variable
name="htmlContent"
type="String"/>
</data>
<RelativeLayout
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="4dp"
android:background="@drawable/bg_blue_card"
android:orientation="vertical">
<TextView
android:id="@+id/tv_contents"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="4dp"
android:gravity="left"
android:padding="8dp"
android:text="@{htmlContent}"/>
</RelativeLayout>
</layout>
32 changes: 32 additions & 0 deletions app/src/main/res/layout/content_list_fragment.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">
<data>
<variable
name="contentFragment"
type="org.oppia.app.player.content.ContentListFragment"/>
<variable
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
name="viewModel"
type="org.oppia.app.player.content.ContentViewModel"/>
</data>
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
tools:context=".player.content.ContentListFragment">
<androidx.recyclerview.widget.RecyclerView
android:id="@+id/recyclerview"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_alignParentTop="true"
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
android:layout_marginStart="8dp"
android:layout_marginTop="4dp"
android:layout_marginEnd="8dp"
android:divider="@android:color/transparent"
android:dividerHeight="8dp"
app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"/>
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
25 changes: 25 additions & 0 deletions app/src/main/res/layout/interation_feedback_card_item.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<layout xmlns:android="http://schemas.android.com/apk/res/android">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For file name, should this be interaction_feedback_card_item?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How shall I name it then, it relates to the learners interaction that should be shown on the right side.

<data>
<variable
name="htmlContent"
type="String"/>
</data>
<RelativeLayout
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginLeft="24dp"
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
android:layout_marginTop="4dp"
android:layout_marginRight="4dp"
android:layout_marginBottom="4dp"
android:orientation="vertical">
<TextView
android:id="@+id/tv_interaction_feedback"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_alignParentRight="true"
android:background="@drawable/bg_white_card"
android:gravity="right|center"
android:padding="8dp"
android:text="@{htmlContent}"/>
</RelativeLayout>
</layout>
Loading