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 #235: Change fragment implementation from directly xml code to better kotlin code. #238

Merged
merged 16 commits into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from 11 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 @@ -12,11 +12,13 @@ const val EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY = "ExplorationActivity.expl
class ExplorationActivity : InjectableAppCompatActivity() {
@Inject
lateinit var explorationActivityPresenter: ExplorationActivityPresenter
private lateinit var explorationId : String

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
activityComponent.inject(this)
explorationActivityPresenter.handleOnCreate()
explorationId = intent.getStringExtra(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY)
explorationActivityPresenter.handleOnCreate(explorationId)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
package org.oppia.app.player.exploration

import android.os.Bundle
import androidx.appcompat.app.AppCompatActivity
import org.oppia.app.R
import org.oppia.app.activity.ActivityScope
import org.oppia.app.home.HomeFragment
import javax.inject.Inject

/** The controller for [ExplorationActivity]. */
@ActivityScope
class ExplorationActivityPresenter @Inject constructor(private val activity: AppCompatActivity) {
fun handleOnCreate() {
fun handleOnCreate(explorationId: String) {
activity.setContentView(R.layout.exploration_activity)
if (getExplorationFragment() == null) {
val explorationFragment = ExplorationFragment()
val args = Bundle()
args.putString(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY, explorationId)
explorationFragment.arguments = args
activity.supportFragmentManager.beginTransaction().add(
R.id.exploration_fragment_placeholder,
ExplorationFragment()
explorationFragment
).commitNow()
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package org.oppia.app.player.exploration

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.Fragment
import org.oppia.app.R
import org.oppia.app.databinding.ExplorationFragmentBinding
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.player.state.StateFragment
import javax.inject.Inject

/** The controller for [ExplorationFragment]. */
Expand All @@ -14,6 +17,22 @@ class ExplorationFragmentPresenter @Inject constructor(
private val fragment: Fragment
) {
fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
return ExplorationFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false).root
val binding = ExplorationFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false).root

if (getStateFragment() == null) {
val stateFragment = StateFragment()
val args = Bundle()
args.putString(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY, fragment.arguments!!.getString(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY))
stateFragment.arguments = args
fragment.childFragmentManager.beginTransaction().add(
R.id.state_fragment_placeholder,
stateFragment
).commitNow()
}
return binding
}

private fun getStateFragment(): StateFragment? {
return fragment.childFragmentManager.findFragmentById(R.id.state_fragment_placeholder) as StateFragment?
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ 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.AudioFragment
import org.oppia.app.player.audio.CellularDataDialogFragment
import org.oppia.app.player.exploration.EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.audio.CellularDialogController
import org.oppia.domain.exploration.ExplorationProgressController
Expand All @@ -20,6 +23,7 @@ import org.oppia.util.logging.Logger
import javax.inject.Inject

private const val TAG_CELLULAR_DATA_DIALOG = "CELLULAR_DATA_DIALOG"
private const val TAG_AUDIO_FRAGMENT = "AUDIO_FRAGMENT"

/** The presenter for [StateFragment]. */
@FragmentScope
Expand All @@ -33,6 +37,7 @@ class StateFragmentPresenter @Inject constructor(

private var showCellularDataDialog = true
private var useCellularData = false
private var explorationId: String? = null

fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
cellularDialogController.getCellularDataPreference()
Expand All @@ -49,6 +54,7 @@ class StateFragmentPresenter @Inject constructor(
it.stateFragment = fragment as StateFragment
it.viewModel = getStateViewModel()
}
explorationId = fragment.arguments!!.getString(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY)

subscribeToCurrentState()

Expand All @@ -57,22 +63,28 @@ class StateFragmentPresenter @Inject constructor(

fun handleAudioClick() {
if (showCellularDataDialog) {
setAudioFragmentVisible(false)
showHideAudioFragment(false)
showCellularDataDialogFragment()
} else {
setAudioFragmentVisible(useCellularData)
if (useCellularData) {
showHideAudioFragment(getAudioFragment() == null)
} else {
showHideAudioFragment(false)
}
}
}

fun handleEnableAudio(saveUserChoice: Boolean) {
setAudioFragmentVisible(true)
if (saveUserChoice)
showHideAudioFragment(true)
if (saveUserChoice) {
cellularDialogController.setAlwaysUseCellularDataPreference()
}
}

fun handleDisableAudio(saveUserChoice: Boolean) {
if (saveUserChoice)
if (saveUserChoice) {
cellularDialogController.setNeverUseCellularDataPreference()
}
}

private fun showCellularDataDialogFragment() {
Expand All @@ -88,8 +100,23 @@ class StateFragmentPresenter @Inject constructor(
return viewModelProvider.getForFragment(fragment, StateViewModel::class.java)
}

fun setAudioFragmentVisible(isVisible: Boolean) {
getStateViewModel().setAudioFragmentVisible(isVisible)
private fun getAudioFragment(): Fragment? {
return fragment.childFragmentManager.findFragmentByTag(TAG_AUDIO_FRAGMENT)
}

private fun showHideAudioFragment(isVisible: Boolean) {
if (isVisible) {
if (getAudioFragment() == null) {
fragment.childFragmentManager.beginTransaction().add(
R.id.audio_fragment_placeholder, AudioFragment(),
TAG_AUDIO_FRAGMENT
).commitNow()
}
} else {
if (getAudioFragment() != null) {
fragment.childFragmentManager.beginTransaction().remove(getAudioFragment()!!).commitNow()
}
}
}

private fun subscribeToCurrentState() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,4 @@ import javax.inject.Inject

/** [ViewModel] for state-fragment. */
@FragmentScope
class StateViewModel @Inject constructor() : ViewModel() {
var isAudioFragmentVisible = ObservableField<Boolean>(false)

fun setAudioFragmentVisible(isVisible: Boolean) {
isAudioFragmentVisible.set(isVisible)
}
}
class StateViewModel @Inject constructor() : ViewModel() {}
7 changes: 2 additions & 5 deletions app/src/main/res/layout/exploration_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="match_parent">

<fragment
android:id="@+id/state_fragment"
class="org.oppia.app.player.state.StateFragment"
<FrameLayout
android:id="@+id/state_fragment_placeholder"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
14 changes: 5 additions & 9 deletions app/src/main/res/layout/state_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,16 @@
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="match_parent">
<LinearLayout
<FrameLayout
android:id="@+id/audio_fragment_placeholder"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginEnd="24dp"
android:visibility="@{viewModel.isAudioFragmentVisible? View.VISIBLE: View.GONE}"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent">
<fragment
android:id="@+id/audio_fragment"
class="org.oppia.app.player.audio.AudioFragment"
android:layout_width="match_parent"
android:layout_height="wrap_content"/>
</LinearLayout>
</FrameLayout>
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
<Button
android:id="@+id/dummy_audio_button"
android:layout_width="wrap_content"
Expand All @@ -42,6 +37,7 @@
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"/>
app:layout_constraintTop_toTopOf="parent"
/>
</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.oppia.util.threading.BackgroundDispatcher
import org.oppia.util.threading.BlockingDispatcher
import javax.inject.Singleton

// TODO(#239): Updated this implementation to creating or removing the AudioFragment only when needed.
veena14cs marked this conversation as resolved.
Show resolved Hide resolved
/** Tests for [StateFragment]. */
@RunWith(AndroidJUnit4::class)
class StateFragmentTest {
Expand Down