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 #1762: NavigationDrawer cancel switch profile always mark home #1816

Closed
Closed
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
d415788
fixed major bugs
prayutsu Sep 5, 2020
519c36a
fixed marking correct last item in navigation drawer
prayutsu Sep 5, 2020
803ce83
fixed nits
prayutsu Sep 5, 2020
e3f336e
fixed lint errors
prayutsu Sep 5, 2020
1931f22
test without any marking
prayutsu Sep 6, 2020
c56d9c4
fixed almost all the bugs
prayutsu Sep 19, 2020
5098c10
tried fixing last bug
prayutsu Sep 21, 2020
0799f75
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Sep 26, 2020
245014b
fixed all the bugs
prayutsu Sep 27, 2020
80a5a8f
removed extra code
prayutsu Sep 28, 2020
f9a8600
Nit change
prayutsu Sep 28, 2020
2f9d2b7
test for reason of admin controls bug
prayutsu Sep 28, 2020
d271dfe
Optimized code
prayutsu Sep 28, 2020
99215f8
fixed lint error
prayutsu Sep 28, 2020
cf488c5
fixed lint error
prayutsu Sep 28, 2020
f5d2fcb
fixed reviewed changes
prayutsu Oct 9, 2020
b071e2b
fixed ktlint failures
prayutsu Oct 9, 2020
56d5250
fixed suggested changes
prayutsu Oct 9, 2020
78e2d62
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Dec 20, 2020
7eccac7
add few test cases
prayutsu Dec 24, 2020
8e4fdc0
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Dec 24, 2020
0ef2f5a
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 6, 2021
bfe6192
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 11, 2021
5d4f8ee
Add more test cases
prayutsu Jan 11, 2021
6639420
Fix imports
prayutsu Jan 11, 2021
0c8b43d
Fix Administrator Controls test cases
prayutsu Jan 12, 2021
007407d
Uncomment All other test cases
prayutsu Jan 12, 2021
05f48f0
Make a function to uncheck all items when Administrator Controls is c…
prayutsu Jan 13, 2021
a1777e5
Fix lint
prayutsu Jan 13, 2021
f9f086c
Fix lint
prayutsu Jan 13, 2021
6c477fc
Improve code
prayutsu Jan 15, 2021
0733811
Replace .isCheckable with .isChecked
prayutsu Jan 16, 2021
72a3fe2
Improve quality of the code by using function
prayutsu Jan 19, 2021
544c814
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 19, 2021
b23869f
Add test cases related to color for all menu items
prayutsu Jan 19, 2021
b03b57e
Restore deleted line
prayutsu Jan 19, 2021
38c09c8
Add @RunOn(TestPlatform.ESPRESSO) notation to added test cases
prayutsu Jan 22, 2021
52ae75d
Fix lint errors
prayutsu Jan 22, 2021
de003c6
Fix suggested changes
prayutsu Jan 22, 2021
08f59d3
Fix lint errors
prayutsu Jan 22, 2021
6fba236
Fix the names of the new test cases
prayutsu Jan 22, 2021
cc45211
Fix suggested changes
prayutsu Jan 22, 2021
72d243c
Fix nits
prayutsu Jan 26, 2021
3774af2
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 26, 2021
4c7f798
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Jan 28, 2021
08deb4b
Correct dialog box dependency
prayutsu Jan 29, 2021
91d75b1
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Feb 2, 2021
6ffd6ed
Replace different keys with a single proto object.
prayutsu Feb 3, 2021
caf0f48
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Feb 3, 2021
93828e2
Fix lint and bazel build errors.
prayutsu Feb 3, 2021
4ff38db
Add ":arguments_java_proto_lite" in the android_library in BUILD.bazel
prayutsu Feb 3, 2021
d65710b
Improve comments' explanation and make the variable names consistent.
prayutsu Feb 3, 2021
381509d
Try implementing suggested changes.
prayutsu Feb 5, 2021
8ec8a2d
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
prayutsu Feb 5, 2021
d9a4608
Fix lint errors.
prayutsu Feb 5, 2021
fe5191f
Make use of enum defined in arguments.proto
prayutsu Feb 5, 2021
b29f8e5
Fix protobuf error.
prayutsu Feb 5, 2021
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 @@ -8,42 +8,58 @@ import androidx.appcompat.app.AlertDialog
import androidx.appcompat.view.ContextThemeWrapper
import androidx.fragment.app.DialogFragment
import org.oppia.android.R
import org.oppia.android.app.model.ExitProfileDialogArguments
import org.oppia.android.app.profile.ProfileChooserActivity
import org.oppia.android.util.extensions.getProto
import org.oppia.android.util.extensions.putProto

/** [DialogFragment] that gives option to either cancel or exit current profile. */
class ExitProfileDialogFragment : DialogFragment() {

companion object {
// TODO(#1655): Re-restrict access to fields in tests post-Gradle.
const val BOOL_IS_FROM_NAVIGATION_DRAWER_EXTRA_KEY =
"BOOL_IS_FROM_NAVIGATION_DRAWER_EXTRA_KEY"
const val EXIT_PROFILE_DIALOG_ARGUMENTS_PROTO = "EXIT_PROFILE_DIALOG_ARGUMENTS_PROTO"

/**
* This function is responsible for displaying content in DialogFragment.
*
* @return [ExitProfileDialogFragment]: DialogFragment
*/
fun newInstance(isFromNavigationDrawer: Boolean): ExitProfileDialogFragment {
fun newInstance(
restoreLastCheckedMenuItem: Boolean,
isAdministratorControlsSelected: Boolean,
lastCheckedMenuItemId: Int
): ExitProfileDialogFragment {
val exitProfileDialogFragment = ExitProfileDialogFragment()
val args = Bundle()
args.putBoolean(BOOL_IS_FROM_NAVIGATION_DRAWER_EXTRA_KEY, isFromNavigationDrawer)
val exitProfileDialogArguments =
ExitProfileDialogArguments.newBuilder()
.setRestoreLastCheckedMenuItem(restoreLastCheckedMenuItem)
.setIsAdministratorControlsSelected(isAdministratorControlsSelected)
.setLastCheckedMenuItemId(lastCheckedMenuItemId).build()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.setLastCheckedMenuItemId(lastCheckedMenuItemId).build()
.setLastCheckedMenuItemId(lastCheckedMenuItemId)
.build()

ktlint doesn't enforce this correctly, but continuation for call chains like this should be broken up such that each operation at the same logical "level" should also be at the same syntactical "level" (meaning on its own line in this case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

args.putProto(EXIT_PROFILE_DIALOG_ARGUMENTS_PROTO, exitProfileDialogArguments)

exitProfileDialogFragment.arguments = args
return exitProfileDialogFragment
}
}

lateinit var exitProfileDialogInterface: ExitProfileDialogInterface
private lateinit var exitProfileDialogInterface: ExitProfileDialogInterface

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val args =
checkNotNull(arguments) { "Expected arguments to be pass to ExitProfileDialogFragment" }

val isFromNavigationDrawer = args.getBoolean(
BOOL_IS_FROM_NAVIGATION_DRAWER_EXTRA_KEY,
false
val exitProfileDialogArguments = args.getProto(
EXIT_PROFILE_DIALOG_ARGUMENTS_PROTO,
ExitProfileDialogArguments.getDefaultInstance()
)

if (isFromNavigationDrawer) {
val restoreLastCheckedMenuItem = exitProfileDialogArguments.restoreLastCheckedMenuItem
val isAdministratorControlsSelected = exitProfileDialogArguments.isAdministratorControlsSelected
val lastCheckedMenuItemId = exitProfileDialogArguments.lastCheckedMenuItemId

if (restoreLastCheckedMenuItem) {
exitProfileDialogInterface =
parentFragment as ExitProfileDialogInterface
}
Expand All @@ -52,15 +68,19 @@ class ExitProfileDialogFragment : DialogFragment() {
.Builder(ContextThemeWrapper(activity as Context, R.style.AlertDialogTheme))
.setMessage(R.string.home_activity_back_dialog_message)
.setNegativeButton(R.string.home_activity_back_dialog_cancel) { dialog, _ ->
if (isFromNavigationDrawer) {
exitProfileDialogInterface.markHomeMenuCloseDrawer()
prayutsu marked this conversation as resolved.
Show resolved Hide resolved
if (restoreLastCheckedMenuItem) {
exitProfileDialogInterface.checkLastCheckedItemAndCloseDrawer(
lastCheckedMenuItemId,
isAdministratorControlsSelected
)
exitProfileDialogInterface.unCheckSwitchProfileItemAndCloseDrawer()
}
dialog.dismiss()
}
.setPositiveButton(R.string.home_activity_back_dialog_exit) { _, _ ->
// TODO(#322): Need to start intent for ProfileChooserActivity to get update. Change to finish when live data bug is fixed.
val intent = ProfileChooserActivity.createProfileChooserActivity(activity!!)
if (!isFromNavigationDrawer) {
if (!restoreLastCheckedMenuItem) {
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
}
activity!!.startActivity(intent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,10 @@ package org.oppia.android.app.drawer

/** Interface to handle option selection in [ExitProfileDialogFragment]. */
interface ExitProfileDialogInterface {
fun markHomeMenuCloseDrawer()
fun checkLastCheckedItemAndCloseDrawer(
lastCheckedMenuItemId: Int,
isAdministratorControlsSelected: Boolean
)

fun unCheckSwitchProfileItemAndCloseDrawer()
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,17 @@ class NavigationDrawerFragment :
navigationDrawerFragmentPresenter.openProfileProgress(profileId)
}

override fun markHomeMenuCloseDrawer() {
navigationDrawerFragmentPresenter.markHomeMenuCloseDrawer()
override fun checkLastCheckedItemAndCloseDrawer(
lastCheckedMenuItemId: Int,
isAdministratorControlsSelected: Boolean
) {
navigationDrawerFragmentPresenter.checkLastCheckedItemAndCloseDrawer(
lastCheckedMenuItemId,
isAdministratorControlsSelected
)
}

override fun unCheckSwitchProfileItemAndCloseDrawer() {
navigationDrawerFragmentPresenter.uncheckSwitchProfileItemAndCloseDrawer()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ class NavigationDrawerFragmentPresenter @Inject constructor(
return@setOnClickListener
}

binding.fragmentDrawerNavView.menu.forEach { menuItem ->
menuItem.isCheckable = false
}
uncheckAllMenuItemsWhenAdministratorControlsIsChecked()

drawerLayout.closeDrawers()
getFooterViewModel().isAdministratorControlsSelected.set(true)
Expand Down Expand Up @@ -199,7 +197,6 @@ class NavigationDrawerFragmentPresenter @Inject constructor(
}

private fun openActivityByMenuItemId(menuItemId: Int) {
getFooterViewModel().isAdministratorControlsSelected.set(false)
if (previousMenuItemId != menuItemId) {
when (NavigationDrawerItem.valueFromNavId(menuItemId)) {
NavigationDrawerItem.HOME -> {
Expand Down Expand Up @@ -244,8 +241,25 @@ class NavigationDrawerFragmentPresenter @Inject constructor(
if (previousFragment != null) {
fragment.childFragmentManager.beginTransaction().remove(previousFragment).commitNow()
}
val lastCheckedMenuItemId: Int
val isAdministratorControlsSelected: Boolean
if (getFooterViewModel().isAdministratorControlsSelected.get() == true) {
getFooterViewModel().isAdministratorControlsSelected.set(false)
isAdministratorControlsSelected = true
lastCheckedMenuItemId = -1
} else {
isAdministratorControlsSelected = false
lastCheckedMenuItemId = previousMenuItemId ?: -1
}
binding.fragmentDrawerNavView.menu.getItem(
NavigationDrawerItem.SWITCH_PROFILE.ordinal
).isChecked = true
val dialogFragment = ExitProfileDialogFragment
.newInstance(isFromNavigationDrawer = true)
.newInstance(
restoreLastCheckedMenuItem = true,
isAdministratorControlsSelected = isAdministratorControlsSelected,
lastCheckedMenuItemId = lastCheckedMenuItemId
)
dialogFragment.showNow(fragment.childFragmentManager, TAG_SWITCH_PROFILE_DIALOG)
}
}
Expand All @@ -263,12 +277,41 @@ class NavigationDrawerFragmentPresenter @Inject constructor(
)
}

fun markHomeMenuCloseDrawer() {
fun checkLastCheckedItemAndCloseDrawer(
lastCheckedMenuItemId: Int,
isAdministratorControlsSelected: Boolean
) {
if (isAdministratorControlsSelected) {
getFooterViewModel().isAdministratorControlsSelected.set(true)
uncheckAllMenuItemsWhenAdministratorControlsIsChecked()
} else if (lastCheckedMenuItemId != -1) {
getFooterViewModel().isAdministratorControlsSelected.set(false)
val checkedMenuItemPosition =
when (lastCheckedMenuItemId) {
NavigationDrawerItem.HOME.value -> 0
NavigationDrawerItem.OPTIONS.value -> 1
NavigationDrawerItem.HELP.value -> 2
NavigationDrawerItem.DOWNLOADS.value -> 3
else -> 4
}
if (checkedMenuItemPosition != 4) {
binding.fragmentDrawerNavView.menu.getItem(checkedMenuItemPosition).isChecked = true
}
}
drawerLayout.closeDrawers()
}

fun uncheckSwitchProfileItemAndCloseDrawer() {
binding.fragmentDrawerNavView.menu.getItem(
NavigationDrawerItem.HOME.ordinal
NavigationDrawerItem.SWITCH_PROFILE.ordinal
).isChecked =
true
drawerLayout.closeDrawers()
false
}

private fun uncheckAllMenuItemsWhenAdministratorControlsIsChecked() {
binding.fragmentDrawerNavView.menu.forEach { item ->
item.isCheckable = false
}
}

/**
Expand Down Expand Up @@ -345,6 +388,7 @@ class NavigationDrawerFragmentPresenter @Inject constructor(
} else {
// For showing navigation drawer in AdministratorControlsActivity
getFooterViewModel().isAdministratorControlsSelected.set(true)
uncheckAllMenuItemsWhenAdministratorControlsIsChecked()
this.drawerLayout = drawerLayout
drawerToggle = object : ActionBarDrawerToggle(
fragment.activity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ class HomeActivity :
supportFragmentManager.beginTransaction().remove(previousFragment).commitNow()
}
val dialogFragment = ExitProfileDialogFragment
.newInstance(isFromNavigationDrawer = false)
.newInstance(
restoreLastCheckedMenuItem = false,
isAdministratorControlsSelected = false,
lastCheckedMenuItemId = -1
)
dialogFragment.showNow(supportFragmentManager, TAG_SWITCH_PROFILE_DIALOG)
}

Expand Down
Loading