-
-
Notifications
You must be signed in to change notification settings - Fork 49
Ccct 2709 confirm backup code profile #3884
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
110fa88
7106faa
1354bdf
1f8d790
44b4060
6d2c7c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="24dp" | ||
| android:height="24dp" | ||
| android:viewportWidth="24" | ||
| android:viewportHeight="24"> | ||
| <path | ||
| android:fillColor="@android:color/white" | ||
| android:pathData="M12.65,10C11.83,7.67 9.61,6 7,6c-3.31,0 -6,2.69 -6,6s2.69,6 6,6c2.61,0 4.83,-1.67 5.65,-4H17v4h4v-4h2v-4H12.65zM7,14c-1.1,0 -2,-0.9 -2,-2s0.9,-2 2,-2 2,0.9 2,2 -0.9,2 -2,2z"/> | ||
| </vector> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package org.commcare.fragments.personalId | ||
|
|
||
| /** | ||
| * Identifies which backup-code flow is being executed. | ||
| * | ||
| * - [CONFIRM_BACKUP_CODE_CHANGE_CODE]: user is on the Manage Profile screen and wants to change | ||
| * their backup code; must confirm the current code first before setting a new one. | ||
| */ | ||
| enum class BackupCodeWorkflow { | ||
| CONFIRM_BACKUP_CODE_CHANGE_CODE, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package org.commcare.fragments.personalId | ||
|
|
||
| import android.os.Bundle | ||
| import android.view.LayoutInflater | ||
| import android.view.View | ||
| import android.view.ViewGroup | ||
| import android.widget.ImageView | ||
| import org.commcare.dalvik.R | ||
| import org.commcare.dalvik.databinding.FragmentRecoveryCodeBinding | ||
| import org.commcare.views.connect.NumericCodeView | ||
|
|
||
| abstract class BasePersonalIdBackupCodeFragment : BasePersonalIdFragment() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This base class isn't doing much beyond inflating the view. I think we should pass the BackupCodeWorkflow mode (e.g., PROFILE_CONFIRM_CODE, PROFILE_CREATE_CODE, SIGNUP_CONFIRM_CODE, SIGNUP_CREATE_CODE) to this base class and configuring the UI (i.e. setting title, hide/show components) directly within this base class for each mode. It should also encapsulate shared behaviors like numeric code listeners rather than duplicating them across every fragment.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Doesn't it defeat the purpose of abstracting a base class if it needs to implement conditional logic based on a passed constant ?
Agree that the common code should be abstracted - 6d2c7c8
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's a valid point, but my suggestion stems from the fact that the base class already inflates the main container view. Since PROFILE_CONFIRM_CODE and SIGNUP_CONFIRM_CODE (Recovery mode) share the exact same view—and PROFILE_CREATE_CODE and SIGNUP_CREATE_CODE (Sign up) share theirs—handling this in the base class will significantly reduce code duplication across individual fragments. |
||
| protected lateinit var binding: FragmentRecoveryCodeBinding | ||
|
|
||
| override fun onCreateView( | ||
| inflater: LayoutInflater, | ||
| container: ViewGroup?, | ||
| savedInstanceState: Bundle?, | ||
| ): View { | ||
| binding = FragmentRecoveryCodeBinding.inflate(inflater, container, false) | ||
| clearBackupCodeFields() | ||
| return binding.root | ||
| } | ||
|
|
||
| override fun onResume() { | ||
| super.onResume() | ||
| binding.backupCodeView.requestFocus(requireActivity()) | ||
| } | ||
|
|
||
| protected fun clearBackupCodeFields() { | ||
| binding.backupCodeView.clearCode() | ||
| binding.confirmCodeView.clearCode() | ||
| } | ||
|
|
||
| protected open fun submitIfEnabled() { | ||
| if (binding.connectBackupCodeButton.isEnabled) handleBackupCodeSubmission() | ||
| } | ||
|
|
||
| abstract fun handleBackupCodeSubmission() | ||
|
|
||
| protected fun togglePasswordVisibility( | ||
| codeView: NumericCodeView, | ||
| toggle: ImageView, | ||
| ) { | ||
| codeView.isPasswordVisible = !codeView.isPasswordVisible | ||
| toggle.setImageResource( | ||
| if (codeView.isPasswordVisible) R.drawable.ic_visibility_off_24 else R.drawable.ic_visibility_24, | ||
| ) | ||
| } | ||
|
|
||
| protected fun clearError() { | ||
| binding.connectBackupCodeErrorMessage.visibility = View.GONE | ||
| binding.connectBackupCodeErrorMessage.text = "" | ||
| } | ||
|
|
||
| protected fun showError(message: String) { | ||
| binding.connectBackupCodeErrorMessage.visibility = View.VISIBLE | ||
| binding.connectBackupCodeErrorMessage.text = message | ||
| } | ||
|
|
||
| protected fun enableContinueButton(enabled: Boolean) { | ||
| binding.connectBackupCodeButton.isEnabled = enabled | ||
| } | ||
|
|
||
| companion object { | ||
| const val BACKUP_CODE_LENGTH = 6 | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be used anywhere right now. Where do we anticipate using this workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll mostly be used for analytics which is part of another ticket, I am happy to remove it for now from the PR if you prefer that.