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

WIP part of #161: Exploration player base 2 contentcard #125

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4908754
Exploration Player base code for activity and fragment
Sep 13, 2019
8e45390
Fake Data Provider
Sep 13, 2019
c6292d8
created dummy content list
veena14cs Sep 13, 2019
77a166d
Update ContentListFragment.kt
veena14cs Sep 13, 2019
451cb3f
Merge remote-tracking branch 'upstream/develop' into HEAD
Sep 16, 2019
10768a3
Nit changes mainly in documentation
Sep 16, 2019
e257140
Nit changes mainly in documentation for State too
Sep 16, 2019
2d7e210
Changed Controller to Presenter and optimised the fragment transactio…
Sep 16, 2019
55c5555
created contentcard and interaction card.
veena14cs Sep 16, 2019
78acf19
Merge branch 'exploration-player-1-base' into exploration-player-2-co…
veena14cs Sep 16, 2019
5639984
removed dummy class
veena14cs Sep 16, 2019
f0ff783
woking on presenter
veena14cs Sep 16, 2019
79bacd9
working on recyclerview data binding
veena14cs Sep 17, 2019
b35d61f
working on adapter
veena14cs Sep 18, 2019
8cb1144
Merge branch 'develop' into exploration-player-2-contentcard
veena14cs Sep 18, 2019
bb23fb7
in progress
veena14cs Sep 18, 2019
4546b61
Updating adpater
veena14cs Sep 19, 2019
6a8cb0a
changing custom adapter to BindableAdapter
veena14cs Sep 19, 2019
996a31f
Creating controller for contentlist
veena14cs Sep 19, 2019
4eec3c8
fixed issues
veena14cs Sep 23, 2019
9e2280c
updated urlimageparser
veena14cs Sep 24, 2019
9a461ab
fetching image from htmlcontent
veena14cs Sep 24, 2019
f66a8c6
updated contentcard
veena14cs Sep 24, 2019
bfa2755
updated and resolved issues
veena14cs Sep 25, 2019
71ff163
fixed styling issues
veena14cs Sep 25, 2019
1cc254a
fixed issues in data module
veena14cs Sep 25, 2019
8118bf9
added todo comment
veena14cs Sep 25, 2019
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
17 changes: 4 additions & 13 deletions .idea/codeStyles/Project.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ dependencies {
'com.google.dagger:dagger:2.24',
"org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version",
"org.jetbrains.kotlinx:kotlinx-coroutines-core:1.2.1",
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.2.1"
"org.jetbrains.kotlinx:kotlinx-coroutines-android:1.2.1",
'androidx.recyclerview:recyclerview:1.0.0',
'com.github.bumptech.glide:glide:4.9.0',
'org.jsoup:jsoup:1.12.1',
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
)
testImplementation(
'androidx.test:core:1.2.0',
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.app">

<uses-permission android:name="android.permission.INTERNET"/>
<application
android:name=".application.OppiaApplication"
android:allowBackup="true"
Expand All @@ -11,6 +12,7 @@
android:roundIcon="@mipmap/ic_launcher_round"
android:supportsRtl="true"
android:theme="@style/OppiaTheme">
<activity android:name=".player.exploration.ExplorationActivity"/>
<activity android:name="org.oppia.app.home.HomeActivity">
<intent-filter>
<action android:name="android.intent.action.MAIN"/>
Expand Down
27 changes: 27 additions & 0 deletions app/src/main/java/org/oppia/app/FakeDataProvider.kt

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import dagger.BindsInstance
import dagger.Subcomponent
import org.oppia.app.fragment.FragmentComponent
import org.oppia.app.home.HomeActivity
import org.oppia.app.player.exploration.ExplorationActivity
import javax.inject.Provider

/** Root subcomponent for all activities. */
Expand All @@ -19,5 +20,6 @@ interface ActivityComponent {

fun getFragmentComponentBuilderProvider(): Provider<FragmentComponent.Builder>

fun inject(explorationActivity: ExplorationActivity)
fun inject(homeActivity: HomeActivity)
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import androidx.fragment.app.Fragment
import dagger.BindsInstance
import dagger.Subcomponent
import org.oppia.app.home.HomeFragment
import org.oppia.app.player.content.ContentListFragment
import org.oppia.app.player.exploration.ExplorationFragment
import org.oppia.app.player.state.StateFragment

/** Root subcomponent for all fragments. */
@Subcomponent
Expand All @@ -15,5 +18,8 @@ interface FragmentComponent {
fun build(): FragmentComponent
}

fun inject(explorationFragment: ExplorationFragment)
fun inject(homeFragment: HomeFragment)
fun inject(stateFragment: StateFragment)
fun inject(contentListFragment: ContentListFragment)
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package org.oppia.app.home

import android.content.Intent
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.core.content.ContextCompat.startActivity
import androidx.fragment.app.Fragment
import kotlinx.android.synthetic.main.home_fragment.view.*
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
import org.oppia.app.databinding.HomeFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.player.exploration.ExplorationActivity
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.UserAppHistoryController
import javax.inject.Inject
Expand All @@ -16,7 +20,7 @@ class HomeFragmentController @Inject constructor(
private val fragment: Fragment,
private val viewModelProvider: ViewModelProvider<UserAppHistoryViewModel>,
private val userAppHistoryController: UserAppHistoryController
) {
){
fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
val binding = HomeFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
// NB: Both the view model and lifecycle owner must be set in order to correctly bind LiveData elements to
Expand All @@ -28,6 +32,10 @@ class HomeFragmentController @Inject constructor(

userAppHistoryController.markUserOpenedApp()

binding.root.btnExploration.setOnClickListener {
val intent = Intent(fragment.requireContext(), ExplorationActivity::class.java)
fragment.requireContext().startActivity(intent) }

return binding.root
}

Expand Down
135 changes: 135 additions & 0 deletions app/src/main/java/org/oppia/app/player/content/ContentCardAdapter.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package org.oppia.app.player.content

import android.content.Context
import android.text.Editable
import android.text.Html
import android.text.Spannable
import android.text.Spanned
import android.util.Log
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.*
import org.oppia.app.R
import org.oppia.app.databinding.ContentCardItemsBinding
import org.oppia.app.databinding.RightInteractionCardItemBinding
import org.oppia.data.backends.gae.model.GaeSubtitledHtml
import org.oppia.util.data.UrlImageParser
import org.xml.sax.Attributes

/** Adapter to bind the HTML contents to the [RecyclerView]. */
class ContentCardAdapter(
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 diffbasing off of #172 and seeing that generic structure simplifies this adapter. I think you could use a ViewModel to perform the Html wrap calls, and then you can just rely on vanilla data-binding methods for inflating the view & binding the view model. That hopefully will result in you not needing this adapter class at all.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I just noticed you did pull in the bindable adapter below--can you update this PR to be diffbased off of #172 to make the history a bit cleaner?

Also, if that adapter works can you remove this one? Feel free to not address any of the comments in this class (minus the one suggesting to use the proto structures instead of the GAE ones), though I do suggest reading through them for your reference.

Copy link
Member

Choose a reason for hiding this comment

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

@veena14cs can we use the bindable adapter here, instead?

private val context: Context,
val contentList: MutableList<GaeSubtitledHtml>
) : RecyclerView.Adapter<RecyclerView.ViewHolder>() {

private val VIEW_TYPE_CONTENT = 1
private val VIEW_TYPE_RIGHT_INTERACTION = 2
Copy link
Member

Choose a reason for hiding this comment

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

What is meant by "right interaction"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is learner's interaction to be displayed on right side of the view

Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't use positional language like 'left/right' since they may be reversed in RTL layouts.

Beyond that, I'm not sure I understand the directionality here. Per the mocks the interaction appears below the content area.


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)
ContentViewHolder(binding)
}
VIEW_TYPE_RIGHT_INTERACTION -> {
val inflater = LayoutInflater.from(parent.getContext())
val binding =
DataBindingUtil.inflate<RightInteractionCardItemBinding>(inflater, R.layout.right_interaction_card_item, parent, false)
RightInteractionViewHolder(binding)
}
else -> throw IllegalArgumentException("Invalid view type")
}
}

override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {

veena14cs marked this conversation as resolved.
Show resolved Hide resolved
when (holder.itemViewType) {
VIEW_TYPE_CONTENT -> (holder as ContentViewHolder).bind(contentList!!.get(position).html)
VIEW_TYPE_RIGHT_INTERACTION -> (holder as RightInteractionViewHolder).bind(contentList!!.get(position).html)
}
}

// Determines the appropriate ViewType according to the sender of the message.
override fun getItemViewType(position: Int): Int {

return if (!contentList!!.get(position).contentId!!.contains("content") && !contentList!!.get(position).contentId!!.contains("Feedback")) {
VIEW_TYPE_RIGHT_INTERACTION
} else {
VIEW_TYPE_CONTENT
}
}

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

inner class ContentViewHolder(val binding: ViewDataBinding) : RecyclerView.ViewHolder(binding.root) {

fun bind(str: String?) {

var htmlContent = str

binding.setVariable(BR.htmlContent, htmlContent)
binding.executePendingBindings();

var CUSTOM_TAG = "oppia-noninteractive-image"
var HTML_TAG = "img"
var CUSTOM_ATTRIBUTE = "filepath-with-value"
var HTML_ATTRIBUTE = "src"

if (htmlContent!!.contains(CUSTOM_TAG)) {

htmlContent = htmlContent.replace(CUSTOM_TAG, HTML_TAG, false);
htmlContent = htmlContent.replace(CUSTOM_ATTRIBUTE, HTML_ATTRIBUTE, false);
htmlContent = htmlContent.replace("&amp;quot;", "")
}

var imageGetter = UrlImageParser(binding.root.tvContents, context)
val html: Spannable
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.N) {
html = Html.fromHtml(htmlContent, Html.FROM_HTML_MODE_LEGACY, imageGetter, null) as Spannable
} else {
html = Html.fromHtml(htmlContent, imageGetter, null) as Spannable
}
binding.root.tvContents.text = html
}
}

inner class RightInteractionViewHolder(val binding: ViewDataBinding) : RecyclerView.ViewHolder(binding.root) {

fun bind(str: String?) {
var htmlContent = str

binding.setVariable(BR.htmlContent, htmlContent)
binding.executePendingBindings();

var CUSTOM_TAG = "oppia-noninteractive-image"
var HTML_TAG = "img"
var CUSTOM_ATTRIBUTE = "filepath-with-value"
var HTML_ATTRIBUTE = "src"

if (htmlContent!!.contains(CUSTOM_TAG)) {

htmlContent = htmlContent.replace(CUSTOM_TAG, HTML_TAG, false);
htmlContent = htmlContent.replace(CUSTOM_ATTRIBUTE, HTML_ATTRIBUTE, false);
htmlContent = htmlContent.replace("&amp;quot;", "")
}

var imageGetter = UrlImageParser(binding.root.tvContents, context)
val html: Spannable
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.N) {
html = Html.fromHtml(htmlContent, Html.FROM_HTML_MODE_LEGACY, imageGetter, null) as Spannable
} else {
html = Html.fromHtml(htmlContent, imageGetter, null) as Spannable
}
binding.root.tvContents.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
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
import retrofit2.Callback
import retrofit2.Response
import java.lang.Exception

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

/** Fragment that displays rich-text content. */
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,67 @@
package org.oppia.app.player.content

import android.content.Context
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.application.ApplicationContext
import org.oppia.app.databinding.ContentListFragmentBinding
import org.oppia.data.backends.gae.model.GaeSubtitledHtml
import javax.inject.Inject

/** Presenter for [ContentListFragment]. */
class ContentListFragmentPresenter @Inject constructor(
@ApplicationContext private val context: Context,
private val fragment: Fragment
) {

private lateinit var binding: ContentListFragmentBinding
var contentCardAdapter: ContentCardAdapter? = null
var contentList: MutableList<GaeSubtitledHtml> = ArrayList()
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned here & elsewhere: let's instead use the structures defined in exploration.proto instead of GAE structures since the UI should never depend on the latter.


fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {

binding = ContentListFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)

binding.recyclerView.apply {

binding.recyclerView.layoutManager = LinearLayoutManager(context)
contentCardAdapter = ContentCardAdapter(context, contentList);
binding.contentCardAdapter = ContentCardAdapter(context, contentList);
}
fetchDummyExplorations()

return binding.root
}

//TODO (#121) :Replace this once interface for ExplorationDataController is available
private fun fetchDummyExplorations() {
veena14cs marked this conversation as resolved.
Show resolved Hide resolved

contentList.add(
GaeSubtitledHtml(
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
"content",
"\u003cp\u003eMeet Matthew!\u003c/p\u003e\u003coppia-noninteractive-image alt-with-value=\"\u0026amp;quot;A boy with a red shirt and blue trousers.\u0026amp;quot;\" caption-with-value=\"\u0026amp;quot;\u0026amp;quot;\" filepath-with-value=\"\u0026amp;quot;img_20180121_113315_pqwqhf863w_height_565_width_343.png\u0026amp;quot;\"\u003e\u003c/oppia-noninteractive-image\u003e\u003cp\u003eMatthew is a young man who likes friends, sports, and eating cake! He also likes learning new things. We\u2019ll follow Matthew as he learns about fractions and how to use them.\u003cbr\u003e\u003c/p\u003e\u003cp\u003eYou should know the following before going on:\u003cbr\u003e\u003c/p\u003e\u003cul\u003e\u003cli\u003eThe counting numbers (1, 2, 3, 4, 5 \u2026.)\u003cbr\u003e\u003c/li\u003e\u003cli\u003eHow to tell whether one counting number is bigger or smaller than another\u003cbr\u003e\u003c/li\u003e\u003c/ul\u003e\u003cp\u003eYou should also get some paper and a pen or pencil to write with, and find a quiet place to work. Take your time, and go through the story at your own pace. Understanding is more important than speed!\u003cbr\u003e\u003c/p\u003e\u003cp\u003eOnce you\u2019re ready, click \u003cstrong\u003eContinue\u003c/strong\u003e to get started!\u003cbr\u003e\u003c/p\u003e"
)
)
contentList.add(
GaeSubtitledHtml(
"content",
"\u003cp\u003e\"OK!\" said Mr. Baker. \"Here's a question for you.\"\u003cbr\u003e\u003c/p\u003e\u003cp\u003e\u003cstrong\u003eQuestion 1\u003c/strong\u003e: If we talk about wanting \u003coppia-noninteractive-math raw_latex-with-value=\"\u0026amp;quot;\\\\frac{4}{7}\u0026amp;quot;\"\u003e\u003c/oppia-noninteractive-math\u003e of a cake, what does the 7 represent?\u003c/p\u003e"
)
)
contentList.add(
GaeSubtitledHtml(
"content", "\u003cp\u003eThe number of pieces of cake I want.\u003c/p\u003e" +
"\u003cp\u003eThe number of pieces the whole cake is cut into.\u003c/p\u003e" +
"\u003cp\u003eI don't remember!\u003c/p\u003e"
)
)
contentList.add(GaeSubtitledHtml("Textinput", "\u003cp\u003eThe number of pieces of cake I want.\u003c/p\u003e"))
contentList.add(GaeSubtitledHtml("Feedback", "\u003cp\u003eNot quite. Let's look at it again.\u003c/p\u003e"))

contentList.add(GaeSubtitledHtml("Textinput", "\u003cp\u003eI don't remember!\u003c/p\u003e"))
contentList.add(GaeSubtitledHtml("Feedback", "\u003cp\u003eThat's OK. Let's look at it again.\u003c/p\u003e"))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.oppia.app.player.exploration

import android.os.Bundle
import org.oppia.app.activity.InjectableAppCompatActivity
import javax.inject.Inject

/** The starting point for explorations. */
class ExplorationActivity : InjectableAppCompatActivity() {

@Inject lateinit var explorationActivityPresenter: ExplorationActivityPresenter

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
activityComponent.inject(this)
explorationActivityPresenter.handleOnCreate()
}
}
Loading