Skip to content

Commit

Permalink
Fix part 135: Topic issues fixes (#285)
Browse files Browse the repository at this point in the history
* multiple tabs

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* cust_args

* test cases,multiple tabs will getting topic id from home
Merge branch 'develop' of https://github.com/oppia/oppia-android into topic-player-multiple-tabs

# Conflicts:
#	app/src/main/AndroidManifest.xml
#	app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt
#	domain/src/main/java/org/oppia/domain/exploration/ExplorationRetriever.kt

* nit

* nit

* nit

* nit

* nit

* separate test cases for all four tabs

* nit

* nit

* nit

* nit

* nit

* nit

* nit

* nit

* Topic Overview see more click and tab switch

* Topic Overview see more click and tab switch

* Topic Overview see more click and tab switch

* topic id todo's are implemented, test cases updated as per new data through topic id

* Topic Overview see more click and tab switch

* nit

* nit

* merged with base branch

* nit

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* revert back to develop

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* set title on the basis of selected topic in topic screen.other nit changes

* nit

* Merge branches 'develop' and 'topic-player-multiple-tabs' of https://github.com/oppia/oppia-android into topic-player-multiple-tabs

# Conflicts:
#	app/src/main/java/org/oppia/app/home/HomeActivity.kt
#	app/src/main/java/org/oppia/app/home/HomeFragmentPresenter.kt
#	app/src/main/java/org/oppia/app/topic/TopicActivity.kt
#	app/src/main/res/layout/home_fragment.xml

* nit

* nit

* nit

* nit

* Merge branches 'develop' and 'topic-player-multiple-tabs' of https://github.com/oppia/oppia-android into topic-player-multiple-tabs

# Conflicts:
#	app/src/main/AndroidManifest.xml

* nit in TopicFragmentPresenter changed var to val in line number 69

* nit

* shorten stackoverflow link,testcase method name change

* nit changes suggested by Rajat

* nit

* nit

* used enum to set current tab in TopicFragmentPresenter.
other nit changes.

* nit

* nit

* nit

* nit

* nit

* nit

* nit

* nit
  • Loading branch information
nikitamarysolomanpvt authored Nov 7, 2019
1 parent 4f6476a commit 790fd64
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class HomeFragmentPresenter @Inject constructor(

private lateinit var topicListAdapter: TopicListAdapter
private lateinit var binding: HomeFragmentBinding

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
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 Down
10 changes: 8 additions & 2 deletions app/src/main/java/org/oppia/app/topic/TopicActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@ 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
@Inject
lateinit var topicActivityPresenter: TopicActivityPresenter

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
activityComponent.inject(this)
topicId = intent?.getStringExtra(org.oppia.app.topic.TOPIC_ACTIVITY_TOPIC_ID_ARGUMENT_KEY) ?: TEST_TOPIC_ID_0
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) ?: ""
topicActivityPresenter.handleOnCreate(topicId)
}

Expand All @@ -37,7 +42,8 @@ class TopicActivity : InjectableAppCompatActivity(), RouteToQuestionPlayerListen
}

override fun routeToTopicPlayFragment() {
// TODO(#135): Change to play tab in this function.
val topicFragment = supportFragmentManager.findFragmentByTag(TOPIC_FRAGMENT_TAG) as TopicFragment
topicFragment.topicFragmentPresenter.setCurrentTab(TopicTab.PLAY)
}

override fun routeToConceptCard(skillId: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.oppia.app.R
import org.oppia.app.activity.ActivityScope
import javax.inject.Inject

const val TOPIC_FRAGMENT_TAG = "TopicFragment"
const val TOPIC_ID_ARGUMENT_KEY = "topic_id"

/** The presenter for [TopicActivity]. */
Expand All @@ -20,7 +21,7 @@ class TopicActivityPresenter @Inject constructor(private val activity: AppCompat
topicFragment.arguments = args
activity.supportFragmentManager.beginTransaction().add(
R.id.topic_fragment_placeholder,
topicFragment
topicFragment, TOPIC_FRAGMENT_TAG
).commitNow()
}
}
Expand Down
24 changes: 14 additions & 10 deletions app/src/main/java/org/oppia/app/topic/TopicFragmentPresenter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import org.oppia.app.R
import org.oppia.app.databinding.TopicFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.Topic
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
Expand All @@ -29,13 +28,14 @@ class TopicFragmentPresenter @Inject constructor(
) {
private lateinit var tabLayout: TabLayout
private lateinit var toolbar: Toolbar
private lateinit var topicId: String
private lateinit var viewPager: ViewPager
private val tabIcons =
intArrayOf(
R.drawable.ic_overview_icon_24dp,
R.drawable.ic_play_icon_24,
R.drawable.ic_train_icon_24,
R.drawable.ic_review_icon_24
R.drawable.ic_play_icon_24dp,
R.drawable.ic_train_icon_24dp,
R.drawable.ic_review_icon_24dp
)

fun handleCreateView(
Expand All @@ -48,19 +48,24 @@ class TopicFragmentPresenter @Inject constructor(
viewPager = binding.root.findViewById(R.id.topic_tabs_viewpager) as ViewPager
tabLayout = binding.root.findViewById(R.id.topic_tabs_container) as TabLayout
toolbar = binding.root.findViewById(R.id.toolbar) as Toolbar
this.topicId = topicId
setUpViewPager(viewPager, topicId)
subscribeToTopicLiveData()
return binding.root
}

fun setCurrentTab(tab: TopicTab) {
viewPager.setCurrentItem(tab.ordinal, true)
}

private fun setUpViewPager(viewPager: ViewPager, topicId: String?) {
val adapter = ViewPagerAdapter(fragment.fragmentManager!!, topicId!!)
viewPager.adapter = adapter
tabLayout.setupWithViewPager(viewPager)
tabLayout.getTabAt(0)!!.setText(TopicTab.getTabForPosition(0).name).setIcon(tabIcons[0])
tabLayout.getTabAt(1)!!.setText(TopicTab.getTabForPosition(1).name).setIcon(tabIcons[1])
tabLayout.getTabAt(2)!!.setText(TopicTab.getTabForPosition(2).name).setIcon(tabIcons[2])
tabLayout.getTabAt(3)!!.setText(TopicTab.getTabForPosition(3).name).setIcon(tabIcons[3])
tabLayout.getTabAt(0)!!.setText(fragment.getString(R.string.overview)).setIcon(tabIcons[0])
tabLayout.getTabAt(1)!!.setText(fragment.getString(R.string.play)).setIcon(tabIcons[1])
tabLayout.getTabAt(2)!!.setText(fragment.getString(R.string.train)).setIcon(tabIcons[2])
tabLayout.getTabAt(3)!!.setText(fragment.getString(R.string.review)).setIcon(tabIcons[3])
}

private val topicLiveData: LiveData<Topic> by lazy { getTopic() }
Expand All @@ -72,9 +77,8 @@ class TopicFragmentPresenter @Inject constructor(
})
}

// TODO(#135): Get this topic-id from [TopicFragment].
private val topicResultLiveData: LiveData<AsyncResult<Topic>> by lazy {
topicController.getTopic(TEST_TOPIC_ID_0)
topicController.getTopic(topicId = topicId)
}

private fun getTopic(): LiveData<Topic> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import org.oppia.app.databinding.TopicOverviewFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.Topic
import org.oppia.app.topic.RouteToTopicPlayListener
import org.oppia.app.topic.TOPIC_ID_ARGUMENT_KEY
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
Expand All @@ -29,10 +29,13 @@ class TopicOverviewFragmentPresenter @Inject constructor(
private val topicController: TopicController
) {
private val routeToTopicPlayListener = activity as RouteToTopicPlayListener

private val topicOverviewViewModel = getTopicOverviewViewModel()
private val topicOverviewViewModel = getTopicOverviewViewModel()
private lateinit var topicId: String

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
topicId = checkNotNull(fragment.arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
"Expected topic ID to be included in arguments for TopicOverviewFragment."
}
val binding = TopicOverviewFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
subscribeToTopicLiveData()
binding.let {
Expand All @@ -59,9 +62,8 @@ class TopicOverviewFragmentPresenter @Inject constructor(
})
}

// TODO(#135): Get this topic-id from [TopicFragment].
private val topicResultLiveData: LiveData<AsyncResult<Topic>> by lazy {
topicController.getTopic(TEST_TOPIC_ID_0)
topicController.getTopic(topicId)
}

private fun getTopicList(): LiveData<Topic> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import org.oppia.app.model.ChapterSummary
import org.oppia.app.model.StorySummary
import org.oppia.app.model.Topic
import org.oppia.app.topic.RouteToStoryListener
import org.oppia.app.topic.TOPIC_ID_ARGUMENT_KEY
import org.oppia.domain.exploration.ExplorationDataController
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
Expand All @@ -37,6 +37,7 @@ class TopicPlayFragmentPresenter @Inject constructor(
private var currentExpandedChapterListIndex: Int? = null

private lateinit var binding: TopicPlayFragmentBinding
private lateinit var topicId: String

private lateinit var expandedChapterListIndexListener: ExpandedChapterListIndexListener

Expand All @@ -46,9 +47,11 @@ class TopicPlayFragmentPresenter @Inject constructor(
currentExpandedChapterListIndex: Int?,
expandedChapterListIndexListener: ExpandedChapterListIndexListener
): View? {
topicId = checkNotNull(fragment.arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
"Expected topic ID to be included in arguments for TopicPlayFragment."
}
this.currentExpandedChapterListIndex = currentExpandedChapterListIndex
this.expandedChapterListIndexListener = expandedChapterListIndexListener

binding = TopicPlayFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
binding.let {
it.lifecycleOwner = fragment
Expand All @@ -59,9 +62,8 @@ class TopicPlayFragmentPresenter @Inject constructor(

private val topicLiveData: LiveData<Topic> by lazy { getTopicList() }

// TODO(#135): Get this topic-id or get storyList from [StoryFragment].
private val topicResultLiveData: LiveData<AsyncResult<Topic>> by lazy {
topicController.getTopic(TEST_TOPIC_ID_0)
topicController.getTopic(topicId)
}

private fun subscribeToTopicLiveData() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.SkillSummary
import org.oppia.app.model.Topic
import org.oppia.app.topic.RouteToConceptCardListener
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import org.oppia.app.topic.TOPIC_ID_ARGUMENT_KEY
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
Expand All @@ -28,14 +28,16 @@ class TopicReviewFragmentPresenter @Inject constructor(
private val logger: Logger,
private val topicController: TopicController
) : ReviewSkillSelector {

private lateinit var topicId: String
private val routeToReviewListener = activity as RouteToConceptCardListener

private lateinit var reviewSkillSelectionAdapter: ReviewSkillSelectionAdapter

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
topicId = checkNotNull(fragment.arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
"Expected topic ID to be included in arguments for TopicReviewFragment."
}
val binding = TopicReviewFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)

reviewSkillSelectionAdapter = ReviewSkillSelectionAdapter(this)
binding.reviewSkillRecyclerView.apply {
adapter = reviewSkillSelectionAdapter
Expand All @@ -55,9 +57,8 @@ class TopicReviewFragmentPresenter @Inject constructor(

private val topicLiveData: LiveData<Topic> by lazy { getTopicList() }

// TODO(#135): Get this topic-id or get skillList from [TopicFragment].
private val topicResultLiveData: LiveData<AsyncResult<Topic>> by lazy {
topicController.getTopic(TEST_TOPIC_ID_0)
topicController.getTopic(topicId)
}

private fun subscribeToTopicLiveData() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import org.oppia.app.databinding.TopicTrainFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.Topic
import org.oppia.app.topic.RouteToQuestionPlayerListener
import org.oppia.app.topic.TOPIC_ID_ARGUMENT_KEY
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.topic.TEST_TOPIC_ID_0
import org.oppia.domain.topic.TopicController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.Logger
Expand All @@ -29,12 +29,14 @@ class TopicTrainFragmentPresenter @Inject constructor(
private val viewModelProvider: ViewModelProvider<TopicTrainViewModel>
) : SkillSelector {
lateinit var selectedSkillIdList: ArrayList<String>

private lateinit var topicId: String
private val routeToQuestionPlayerListener = activity as RouteToQuestionPlayerListener

private lateinit var skillSelectionAdapter: SkillSelectionAdapter

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?, skillList: ArrayList<String>): View? {
topicId = checkNotNull(fragment.arguments?.getString(TOPIC_ID_ARGUMENT_KEY)) {
"Expected topic ID to be included in arguments for TopicTrainFragment."
}
selectedSkillIdList = skillList
val binding = TopicTrainFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)

Expand All @@ -52,9 +54,8 @@ class TopicTrainFragmentPresenter @Inject constructor(

private val topicLiveData: LiveData<Topic> by lazy { getTopicList() }

// TODO(#135): Get this topic-id or get skillList from [TopicFragment].
private val topicResultLiveData: LiveData<AsyncResult<Topic>> by lazy {
topicController.getTopic(TEST_TOPIC_ID_0)
topicController.getTopic(topicId)
}

private fun subscribeToTopicLiveData() {
Expand Down
7 changes: 6 additions & 1 deletion app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@
<string name="state_end_exploration_button">RETURN TO TOPIC</string>
<string name="state_learn_again_button">LEARN AGAIN</string>
<string name="topic_overview_see_more">See More</string>
<string name="topic_download_text"> (%s MB)</string>
<string name="topic_download_text">(%s MB)</string>
<string name="overview">Overview</string>
<string name="play">Play</string>
<string name="train">Train</string>
<string name="review">Review</string>
<string name="topic">Topic</string>
<string name="topic_story_progress_percentage">%s\%%</string>
<string name="fractions_hint_text">Write fraction here.</string>
<string name="number_input_hint_text">Write numbers here.</string>
Expand Down
11 changes: 11 additions & 0 deletions app/src/sharedTest/java/org/oppia/app/topic/TopicFragmentTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import android.content.Context
import androidx.test.core.app.ActivityScenario.launch
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.action.ViewActions.scrollTo
import androidx.test.espresso.action.ViewActions.swipeLeft
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA
Expand Down Expand Up @@ -301,6 +302,16 @@ class TopicFragmentTest {
)
}
}
@Test
fun testTopicActivity_clickOnSeeMore_isPlayTabIsSelectedAndContentMatched() {
launch(TopicActivity::class.java).use {
onView(
withId(R.id.see_more_text_view)
).perform(scrollTo(), click())
onView(withId(R.id.topic_tabs_container)).check(matches(matchCurrentTabTitle("PLAY")))
onView(withText("First Story")).check(matches(isDisplayed()))
}
}

@Module
class TestModule {
Expand Down

0 comments on commit 790fd64

Please sign in to comment.