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

Fixes #1198 & 1200: Options - Tablet (Landscape) (Portrait) (Lowfi) #1677

Merged
merged 23 commits into from
Aug 29, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c6f7dd3
-Implemented multipane options (with some issues)
MohamedMedhat1998 Aug 18, 2020
006001f
-Fixed the issues that was facing the "OptionsFragment"
MohamedMedhat1998 Aug 18, 2020
379cdb4
-Added a TextView for the title of the multipane options
MohamedMedhat1998 Aug 18, 2020
08d0f51
-Added test cases for "OptionsFragment"
MohamedMedhat1998 Aug 19, 2020
cbba5db
-Added test cases for "AppLanguage", "DefaultAudio", and "StoryTextSize"
MohamedMedhat1998 Aug 19, 2020
5a37741
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 19, 2020
a0e6089
-Fixed ktlint issues
MohamedMedhat1998 Aug 19, 2020
809bbcd
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 20, 2020
838e523
-Some nit changes
MohamedMedhat1998 Aug 20, 2020
20909c0
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 22, 2020
12f5577
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 22, 2020
cb17310
-updated dagger modules in "AppLanguageFragmentTest"
MohamedMedhat1998 Aug 24, 2020
f2f0fbf
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 24, 2020
13b4857
-updated the failing test cases
MohamedMedhat1998 Aug 28, 2020
b2e89a7
-some refactoring
MohamedMedhat1998 Aug 28, 2020
c119e7a
Merge remote-tracking branch 'origin/develop' into options-tablet-lowfi
MohamedMedhat1998 Aug 28, 2020
56ba897
-Fixed lint issues
MohamedMedhat1998 Aug 28, 2020
c2239a7
-Added "Options" listeners to BUILD.bazel
MohamedMedhat1998 Aug 28, 2020
9c1861f
-Updated "OptionsFragmentPresenter" "update" methods implementation
MohamedMedhat1998 Aug 28, 2020
b93f163
-Fixed ktlint issues
MohamedMedhat1998 Aug 28, 2020
f060712
-Added logs in the case of errors in "OptionsFragmentPresenter"
MohamedMedhat1998 Aug 28, 2020
0981187
-updated the error logs in "OptionsFragmentPresenter"
MohamedMedhat1998 Aug 29, 2020
8114802
-ktlint issues
MohamedMedhat1998 Aug 29, 2020
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 @@ -32,8 +32,7 @@ class AppLanguageFragmentPresenter @Inject constructor(private val fragment: Fra
adapter = languageSelectionAdapter
}

// TODO(#1200): Stop the toolbar functionality in the multipane (add non-null receiver (?)).
binding.appLanguageToolbar.setNavigationOnClickListener {
binding.appLanguageToolbar?.setNavigationOnClickListener {
val message = languageSelectionAdapter.getSelectedLanguage()
val intent = Intent()
intent.putExtra(KEY_MESSAGE_APP_LANGUAGE, message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class DefaultAudioFragmentPresenter @Inject constructor(private val fragment: Fr
adapter = languageSelectionAdapter
}

// TODO(#1200): Stop the toolbar functionality in the multipane (add non-null receiver (?)).
binding.audioLanguageToolbar.setNavigationOnClickListener {
binding.audioLanguageToolbar?.setNavigationOnClickListener {
val message = languageSelectionAdapter.getSelectedLanguage()
val intent = Intent()
intent.putExtra(KEY_MESSAGE_AUDIO_LANGUAGE, message)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.options

/** Listener for when an activity should load a [AppLanguageFragment]. */
interface LoadAppLanguageListListener {
fun loadAppLanguageFragment(appLanguage: String)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.options

/** Listener for when an activity should load a [DefaultAudioFragment]. */
interface LoadAudioLanguageListListener {
fun loadAudioLanguageFragment(audioLanguage: String)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.oppia.app.options

/** Listener for when an activity should load a [StoryTextSizeFragment]. */
interface LoadStoryTextSizeListener {
fun loadStoryTextSizeFragment(textSize: String)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.oppia.app.options
import androidx.appcompat.app.AppCompatActivity
import androidx.databinding.ObservableList
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Transformations
import androidx.lifecycle.ViewModel
import org.oppia.app.fragment.FragmentScope
Expand All @@ -29,6 +30,19 @@ class OptionControlsViewModel @Inject constructor(
private val routeToStoryTextSizeListener = activity as RouteToStoryTextSizeListener
private val routeToAudioLanguageListListener = activity as RouteToAudioLanguageListListener
private val routeToAppLanguageListListener = activity as RouteToAppLanguageListListener
private val loadStoryTextSizeListener = activity as LoadStoryTextSizeListener
private val loadAudioLanguageListListener = activity as LoadAudioLanguageListListener
private val loadAppLanguageListListener = activity as LoadAppLanguageListListener
private var isFirstOpen = true
val uiLiveData = MutableLiveData<Boolean>()

/**
* Should be called with `false` when the UI starts to load, then with `true` after the UI
* finishes loading.
*/
fun isUIInitialized(isInitialized: Boolean) {
uiLiveData.value = isInitialized
}

private val profileResultLiveData: LiveData<AsyncResult<Profile>> by lazy {
profileManagementController.getProfile(profileId)
Expand All @@ -48,6 +62,10 @@ class OptionControlsViewModel @Inject constructor(
this.profileId = profileId
}

fun getProfileId(): ProfileId {
return this.profileId
}

private fun processProfileResult(profile: AsyncResult<Profile>): Profile {
if (profile.isFailure()) {
logger.e("OptionsFragment", "Failed to retrieve profile", profile.getErrorOrNull()!!)
Expand All @@ -60,11 +78,14 @@ class OptionControlsViewModel @Inject constructor(
itemViewModelList.clear()

val optionsStoryTextSizeViewModel =
OptionsStoryTextSizeViewModel(routeToStoryTextSizeListener)
OptionsStoryTextSizeViewModel(routeToStoryTextSizeListener, loadStoryTextSizeListener)
val optionsAppLanguageViewModel =
OptionsAppLanguageViewModel(routeToAppLanguageListListener)
OptionsAppLanguageViewModel(routeToAppLanguageListListener, loadAppLanguageListListener)
val optionAudioViewViewModel =
OptionsAudioLanguageViewModel(routeToAudioLanguageListListener)
OptionsAudioLanguageViewModel(
routeToAudioLanguageListListener,
loadAudioLanguageListListener
)

optionsStoryTextSizeViewModel.storyTextSize.set(getStoryTextSize(profile.storyTextSize))
optionsAppLanguageViewModel.appLanguage.set(getAppLanguage(profile.appLanguage))
Expand All @@ -76,9 +97,23 @@ class OptionControlsViewModel @Inject constructor(

itemViewModelList.add(optionAudioViewViewModel as OptionsItemViewModel)

// Loading the initial options in the sub-options container
if (isMultipane.get()!! && isFirstOpen) {
optionsStoryTextSizeViewModel.loadStoryTextSizeFragment()
isFirstOpen = false
}

return itemViewModelList
}

/**
* Used to set [isFirstOpen] value which controls the loading of the initial extra-option fragment
* in the case of multipane.
*/
fun isFirstOpen(isFirstOpen: Boolean) {
this.isFirstOpen = isFirstOpen
}

fun getStoryTextSize(storyTextSize: StoryTextSize): String {
return when (storyTextSize) {
StoryTextSize.SMALL_TEXT_SIZE -> "Small"
Expand Down
39 changes: 37 additions & 2 deletions app/src/main/java/org/oppia/app/options/OptionsActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,27 @@ package org.oppia.app.options
import android.content.Context
import android.content.Intent
import android.os.Bundle
import android.widget.TextView
import org.oppia.app.R
import org.oppia.app.activity.InjectableAppCompatActivity
import org.oppia.app.drawer.KEY_NAVIGATION_PROFILE_ID
import javax.inject.Inject

private const val SELECTED_OPTIONS_TITLE_KEY = "SELECTED_OPTIONS_TITLE_KEY"

/** The activity for setting user preferences. */
class OptionsActivity :
InjectableAppCompatActivity(),
RouteToAppLanguageListListener,
RouteToAudioLanguageListListener,
RouteToStoryTextSizeListener {
RouteToStoryTextSizeListener,
LoadStoryTextSizeListener,
LoadAppLanguageListListener,
LoadAudioLanguageListListener {
@Inject
lateinit var optionActivityPresenter: OptionsActivityPresenter
// used to initially load the suitable fragment in the case of multipane.
private var isFirstOpen = true

companion object {
// TODO(#1655): Re-restrict access to fields in tests post-Gradle.
Expand All @@ -41,7 +49,11 @@ class OptionsActivity :
BOOL_IS_FROM_NAVIGATION_DRAWER_EXTRA_KEY,
/* defaultValue= */ false
)
optionActivityPresenter.handleOnCreate(isFromNavigationDrawer)
if (savedInstanceState != null) {
isFirstOpen = false
}
val extraOptionsTitle = savedInstanceState?.getString(SELECTED_OPTIONS_TITLE_KEY)
optionActivityPresenter.handleOnCreate(isFromNavigationDrawer, extraOptionsTitle, isFirstOpen)
title = getString(R.string.menu_options)
}

Expand Down Expand Up @@ -95,4 +107,27 @@ class OptionsActivity :
REQUEST_CODE_TEXT_SIZE
)
}

override fun loadStoryTextSizeFragment(textSize: String) {
optionActivityPresenter.setExtraOptionTitle(getString(R.string.story_text_size))
optionActivityPresenter.loadStoryTextSizeFragment(textSize)
}

override fun loadAppLanguageFragment(appLanguage: String) {
optionActivityPresenter.setExtraOptionTitle(getString(R.string.app_language))
optionActivityPresenter.loadAppLanguageFragment(appLanguage)
}

override fun loadAudioLanguageFragment(audioLanguage: String) {
optionActivityPresenter.setExtraOptionTitle(getString(R.string.audio_language))
optionActivityPresenter.loadAudioLanguageFragment(audioLanguage)
}

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
val titleTextView = findViewById<TextView>(R.id.options_activity_selected_options_title)
if (titleTextView != null) {
outState.putString(SELECTED_OPTIONS_TITLE_KEY, titleTextView.text.toString())
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.oppia.app.options

import android.view.View
import android.widget.FrameLayout
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.widget.Toolbar
import androidx.drawerlayout.widget.DrawerLayout
Expand All @@ -17,8 +19,17 @@ class OptionsActivityPresenter @Inject constructor(
private var navigationDrawerFragment: NavigationDrawerFragment? = null
private lateinit var toolbar: Toolbar

fun handleOnCreate(isFromNavigationDrawer: Boolean) {
fun handleOnCreate(
isFromNavigationDrawer: Boolean,
extraOptionsTitle: String?,
isFirstOpen: Boolean
) {
activity.setContentView(R.layout.option_activity)
val titleTextView =
activity.findViewById<TextView>(R.id.options_activity_selected_options_title)
if (titleTextView != null) {
titleTextView.text = extraOptionsTitle
}
setUpToolbar()
if (isFromNavigationDrawer) {
setUpNavigationDrawer()
Expand All @@ -28,12 +39,15 @@ class OptionsActivityPresenter @Inject constructor(
activity.finish()
}
}
if (getOptionFragment() == null) {
activity.supportFragmentManager.beginTransaction().add(
R.id.options_fragment_placeholder,
OptionsFragment()
).commitNow()
val isMultipane = activity.findViewById<FrameLayout>(R.id.multipane_options_container) != null
val previousFragment = getOptionFragment()
if (previousFragment != null) {
activity.supportFragmentManager.beginTransaction().remove(previousFragment).commitNow()
}
activity.supportFragmentManager.beginTransaction().add(
R.id.options_fragment_placeholder,
OptionsFragment.newInstance(isMultipane, isFirstOpen)
).commitNow()
}

private fun setUpToolbar() {
Expand Down Expand Up @@ -73,4 +87,34 @@ class OptionsActivityPresenter @Inject constructor(
fun updateAudioLanguage(audioLanguage: String) {
getOptionFragment()?.updateAudioLanguage(audioLanguage)
}

fun loadStoryTextSizeFragment(textSize: String) {
val storyTextSizeFragment = StoryTextSizeFragment.newInstance(textSize)
activity.supportFragmentManager
.beginTransaction()
.add(R.id.multipane_options_container, storyTextSizeFragment)
.commitNow()
}

fun loadAppLanguageFragment(appLanguage: String) {
val appLanguageFragment =
AppLanguageFragment.newInstance(APP_LANGUAGE, appLanguage)
activity.supportFragmentManager
.beginTransaction()
.add(R.id.multipane_options_container, appLanguageFragment)
.commitNow()
}

fun loadAudioLanguageFragment(audioLanguage: String) {
val defaultAudioFragment =
DefaultAudioFragment.newInstance(AUDIO_LANGUAGE, audioLanguage)
activity.supportFragmentManager
.beginTransaction()
.add(R.id.multipane_options_container, defaultAudioFragment)
.commitNow()
}

fun setExtraOptionTitle(title: String) {
activity.findViewById<TextView>(R.id.options_activity_selected_options_title).text = title
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import androidx.databinding.ObservableField

/** App language settings view model for the recycler view in [OptionsFragment]. */
class OptionsAppLanguageViewModel(
private val routeToAppLanguageListListener: RouteToAppLanguageListListener
private val routeToAppLanguageListListener: RouteToAppLanguageListListener,
private var loadAppLanguageListListener: LoadAppLanguageListListener
) : OptionsItemViewModel() {
val appLanguage = ObservableField<String>("")

Expand All @@ -13,6 +14,10 @@ class OptionsAppLanguageViewModel(
}

fun onAppLanguageClicked() {
routeToAppLanguageListListener.routeAppLanguageList(appLanguage.get())
if (isMultipane.get()!!) {
loadAppLanguageListListener.loadAppLanguageFragment(appLanguage.get()!!)
} else {
routeToAppLanguageListListener.routeAppLanguageList(appLanguage.get())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import androidx.databinding.ObservableField

/** Audio language settings view model for the recycler view in [OptionsFragment]. */
class OptionsAudioLanguageViewModel(
private val routeToAudioLanguageListListener: RouteToAudioLanguageListListener
private val routeToAudioLanguageListListener: RouteToAudioLanguageListListener,
private val loadAudioLanguageListListener: LoadAudioLanguageListListener
) : OptionsItemViewModel() {
val audioLanguage = ObservableField<String>("")

Expand All @@ -13,6 +14,10 @@ class OptionsAudioLanguageViewModel(
}

fun onAudioLanguageClicked() {
routeToAudioLanguageListListener.routeAudioLanguageList(audioLanguage.get())
if (isMultipane.get()!!) {
loadAudioLanguageListListener.loadAudioLanguageFragment(audioLanguage.get()!!)
} else {
routeToAudioLanguageListListener.routeAudioLanguageList(audioLanguage.get())
}
}
}
32 changes: 28 additions & 4 deletions app/src/main/java/org/oppia/app/options/OptionsFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,25 @@ const val REQUEST_CODE_TEXT_SIZE = 1
const val REQUEST_CODE_APP_LANGUAGE = 2
const val REQUEST_CODE_AUDIO_LANGUAGE = 3

private const val IS_MULTIPANE_EXTRA = "IS_MULTIPANE"
private const val IS_FIRST_OPEN_EXTRA = "IS_FIRST_OPEN_EXTRA"

/** Fragment that contains an introduction to the app. */
class OptionsFragment : InjectableFragment() {
@Inject
lateinit var optionsFragmentPresenter: OptionsFragmentPresenter

companion object {
fun newInstance(isMultipane: Boolean, isFirstOpen: Boolean): OptionsFragment {
val args = Bundle()
args.putBoolean(IS_MULTIPANE_EXTRA, isMultipane)
args.putBoolean(IS_FIRST_OPEN_EXTRA, isFirstOpen)
val fragment = OptionsFragment()
fragment.arguments = args
return fragment
}
}

override fun onAttach(context: Context) {
super.onAttach(context)
fragmentComponent.inject(this)
Expand All @@ -30,18 +44,28 @@ class OptionsFragment : InjectableFragment() {
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
return optionsFragmentPresenter.handleCreateView(inflater, container)
val args =
checkNotNull(arguments) { "Expected arguments to be passed to OptionsFragment" }
val isMultipane = args.getBoolean(IS_MULTIPANE_EXTRA)
val isFirstOpen = args.getBoolean(IS_FIRST_OPEN_EXTRA)
return optionsFragmentPresenter.handleCreateView(inflater, container, isMultipane, isFirstOpen)
}

fun updateStoryTextSize(textSize: String) {
optionsFragmentPresenter.updateStoryTextSize(textSize)
optionsFragmentPresenter.runAfterUIInitialization {
optionsFragmentPresenter.updateStoryTextSize(textSize)
}
}

fun updateAppLanguage(appLanguage: String) {
optionsFragmentPresenter.updateAppLanguage(appLanguage)
optionsFragmentPresenter.runAfterUIInitialization {
optionsFragmentPresenter.updateAppLanguage(appLanguage)
}
}

fun updateAudioLanguage(audioLanguage: String) {
optionsFragmentPresenter.updateAudioLanguage(audioLanguage)
optionsFragmentPresenter.runAfterUIInitialization {
optionsFragmentPresenter.updateAudioLanguage(audioLanguage)
}
}
}
Loading