-
Notifications
You must be signed in to change notification settings - Fork 46
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
[New Designs] Implement add link flow #642
Changes from 53 commits
f1392aa
5ac4479
4d0a80d
631230a
1f813af
9b41077
059ef51
66ffbc3
09d2685
9353e64
657a6c9
6fd4edf
d532158
9e4ea1d
9bbe78d
d1c53cd
de8667b
1c9db05
c4d1e68
9309eff
51f350e
1461e29
1c5a84a
20a3160
5fbc024
3902641
2044002
30d799b
8c1ab25
2d4437d
c2376ec
4b51010
1b0a716
c5a2db4
31f8d4c
80731aa
b39b639
c51ebb0
d2dd31b
369adfa
1b254ee
599fe32
b13c5a0
b7b4536
d9ca5df
e1b15b8
4e0d850
44b4ac5
aa702c5
86342f1
d3bd5d7
5a42afb
44ea86d
dad3f90
788d22a
b439010
d5c8531
db37b06
ae929c8
8375440
5feeaab
bd68a91
d096681
1b2ac66
4bf6085
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,191 @@ | ||||||||||
package org.openobservatory.ooniprobe.activity.add_descriptor | ||||||||||
|
||||||||||
import android.content.Context | ||||||||||
import android.content.Intent | ||||||||||
import android.os.Build.VERSION | ||||||||||
import android.os.Build.VERSION_CODES | ||||||||||
import android.os.Bundle | ||||||||||
import android.view.Menu | ||||||||||
import android.view.MenuInflater | ||||||||||
import android.view.MenuItem | ||||||||||
import android.widget.ImageView | ||||||||||
import android.widget.TextView | ||||||||||
import androidx.activity.viewModels | ||||||||||
import androidx.appcompat.widget.Toolbar | ||||||||||
import androidx.databinding.BindingAdapter | ||||||||||
import androidx.lifecycle.ViewModel | ||||||||||
import androidx.lifecycle.ViewModelProvider | ||||||||||
import com.google.android.material.checkbox.MaterialCheckBox | ||||||||||
import io.noties.markwon.Markwon | ||||||||||
import org.openobservatory.engine.OONIRunDescriptor | ||||||||||
import org.openobservatory.ooniprobe.R | ||||||||||
import org.openobservatory.ooniprobe.activity.AbstractActivity | ||||||||||
import org.openobservatory.ooniprobe.activity.add_descriptor.adapter.AddDescriptorExpandableListAdapter | ||||||||||
import org.openobservatory.ooniprobe.activity.add_descriptor.adapter.GroupedItem | ||||||||||
import org.openobservatory.ooniprobe.common.PreferenceManager | ||||||||||
import org.openobservatory.ooniprobe.common.ReadMorePlugin | ||||||||||
import org.openobservatory.ooniprobe.common.TestDescriptorManager | ||||||||||
import org.openobservatory.ooniprobe.databinding.ActivityAddDescriptorBinding | ||||||||||
import javax.inject.Inject | ||||||||||
|
||||||||||
/** | ||||||||||
* This activity is used to add a new descriptor to the application. The activity shows the tests that are included in the descriptor. | ||||||||||
* The user can select which tests to include, and if the descriptor should be automatically updated. | ||||||||||
*/ | ||||||||||
class AddDescriptorActivity : AbstractActivity() { | ||||||||||
aanorbel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
companion object { | ||||||||||
private const val DESCRIPTOR = "descriptor" | ||||||||||
|
||||||||||
/** | ||||||||||
* This method is used to create an intent to start this activity. | ||||||||||
* @param context is the context of the activity that calls this method | ||||||||||
* @param descriptor is the descriptor to add | ||||||||||
* @return an intent to start this activity | ||||||||||
*/ | ||||||||||
@JvmStatic | ||||||||||
fun newIntent(context: Context, descriptor: OONIRunDescriptor): Intent { | ||||||||||
return Intent(context, AddDescriptorActivity::class.java).putExtra( | ||||||||||
DESCRIPTOR, | ||||||||||
descriptor | ||||||||||
) | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* This method is used to set the text of a textview as markdown. The markdown is parsed using the markwon library. | ||||||||||
* The textview must have the attribute app:richText set to the markdown text to parse. | ||||||||||
* @param view is the textview that will contain the parsed text | ||||||||||
* @param richText is the markdown text to parse | ||||||||||
*/ | ||||||||||
@JvmStatic | ||||||||||
@BindingAdapter(value = ["richText"]) | ||||||||||
fun setRichText(view: TextView, richText: String?) { | ||||||||||
richText?.let { textValue -> | ||||||||||
val r = view.context.resources | ||||||||||
val markwon = Markwon.builder(view.context) | ||||||||||
.usePlugin( | ||||||||||
ReadMorePlugin( | ||||||||||
labelMore = r.getString(R.string.OONIRun_ReadMore), | ||||||||||
labelLess = r.getString(R.string.OONIRun_ReadLess) | ||||||||||
) | ||||||||||
) | ||||||||||
.build() | ||||||||||
markwon.setMarkdown(view, textValue) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* This method is used to set the image of an imageview as a drawable resource. The drawable is set using the name of the resource. | ||||||||||
* The imageview must have the attribute app:resource set to the name of the resource to set. | ||||||||||
* @param imageView is the imageview that will contain the drawable resource | ||||||||||
* @param iconName is the name of the drawable resource | ||||||||||
*/ | ||||||||||
@JvmStatic | ||||||||||
@BindingAdapter(value = ["resource"]) | ||||||||||
fun setImageViewResource(imageView: ImageView, iconName: String?) { | ||||||||||
/*TODO(aanorbel): Update to parse the icon name and set the correct icon. | ||||||||||
* Remember to ignore icons generated when generated doing this.*/ | ||||||||||
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.
Suggested change
|
||||||||||
imageView.setImageResource(R.drawable.ooni_empty_state) | ||||||||||
} | ||||||||||
|
||||||||||
} | ||||||||||
|
||||||||||
@Inject | ||||||||||
lateinit var preferenceManager: PreferenceManager | ||||||||||
|
||||||||||
@Inject | ||||||||||
lateinit var descriptorManager: TestDescriptorManager | ||||||||||
|
||||||||||
aanorbel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
override fun onCreate(savedInstanceState: Bundle?) { | ||||||||||
super.onCreate(savedInstanceState) | ||||||||||
activityComponent.inject(this) | ||||||||||
val binding = ActivityAddDescriptorBinding.inflate(layoutInflater) | ||||||||||
setContentView(binding.root) | ||||||||||
setSupportActionBar(binding.toolbar) | ||||||||||
supportActionBar?.setDisplayHomeAsUpEnabled(false) | ||||||||||
supportActionBar?.setDisplayShowHomeEnabled(false) | ||||||||||
supportActionBar?.title = "Add New Link" | ||||||||||
val descriptorExtra = if (VERSION.SDK_INT >= VERSION_CODES.TIRAMISU) { | ||||||||||
intent.getParcelableExtra(DESCRIPTOR, OONIRunDescriptor::class.java) | ||||||||||
} else { | ||||||||||
intent.getSerializableExtra(DESCRIPTOR) as OONIRunDescriptor? | ||||||||||
} | ||||||||||
val viewModel: AddDescriptorViewModel by viewModels { | ||||||||||
object : ViewModelProvider.Factory { | ||||||||||
override fun <T : ViewModel> create(modelClass: Class<T>): T { | ||||||||||
return AddDescriptorViewModel(descriptorManager) as T | ||||||||||
} | ||||||||||
} | ||||||||||
} | ||||||||||
binding.viewmodel = viewModel | ||||||||||
binding.lifecycleOwner = this | ||||||||||
descriptorExtra?.let { descriptor -> | ||||||||||
viewModel.onDescriptorChanged(descriptor) | ||||||||||
val adapter = AddDescriptorExpandableListAdapter( | ||||||||||
nettests = descriptor.nettests.map { nettest -> | ||||||||||
GroupedItem( | ||||||||||
name = nettest.name, | ||||||||||
inputs = nettest.inputs, | ||||||||||
selected = true | ||||||||||
) | ||||||||||
}, | ||||||||||
viewModel = viewModel | ||||||||||
) | ||||||||||
binding.expandableListView.setAdapter(adapter) | ||||||||||
for (i in 0 until adapter.groupCount) { | ||||||||||
binding.expandableListView.expandGroup(i) | ||||||||||
} | ||||||||||
val bottomBarOnMenuItemClickListener: Toolbar.OnMenuItemClickListener = | ||||||||||
Toolbar.OnMenuItemClickListener { item -> | ||||||||||
when (item.itemId) { | ||||||||||
R.id.add_descriptor -> { | ||||||||||
viewModel.onAddButtonClicked( | ||||||||||
selectedNettest = adapter.nettests.filter { it.selected }, | ||||||||||
automatedUpdates = binding.automaticUpdatesSwitch.isChecked | ||||||||||
) | ||||||||||
true | ||||||||||
} | ||||||||||
|
||||||||||
else -> false | ||||||||||
} | ||||||||||
} | ||||||||||
binding.bottomBar.setOnMenuItemClickListener(bottomBarOnMenuItemClickListener) | ||||||||||
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. Maybe split this long block of code by adding blank lines and brief comments? |
||||||||||
|
||||||||||
viewModel.selectedAllBtnStatus.observe(this) { state -> | ||||||||||
binding.testsCheckbox.checkedState = state; | ||||||||||
} | ||||||||||
|
||||||||||
// This observer is used to change the state of the "Select All" button when a checkbox is clicked. | ||||||||||
binding.testsCheckbox.addOnCheckedStateChangedListener { checkBox, state -> | ||||||||||
viewModel.setSelectedAllBtnStatus(state) | ||||||||||
adapter.notifyDataSetChanged() | ||||||||||
} | ||||||||||
|
||||||||||
// This observer is used to finish the activity when the descriptor is added. | ||||||||||
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 kind of documentation is much appreciated! I would like to encourage you to do add more documentation like this one especially when writing new code! |
||||||||||
viewModel.finishActivity.observe(this) { shouldFinish -> | ||||||||||
if (shouldFinish) { | ||||||||||
finish() | ||||||||||
} | ||||||||||
} | ||||||||||
} ?: run { | ||||||||||
finish() | ||||||||||
} | ||||||||||
|
||||||||||
} | ||||||||||
|
||||||||||
override fun onCreateOptionsMenu(menu: Menu): Boolean { | ||||||||||
val inflater: MenuInflater = menuInflater | ||||||||||
inflater.inflate(R.menu.close, menu) | ||||||||||
return true | ||||||||||
} | ||||||||||
|
||||||||||
override fun onOptionsItemSelected(item: MenuItem): Boolean { | ||||||||||
return when (item.itemId) { | ||||||||||
R.id.close_button -> { | ||||||||||
finish() | ||||||||||
true | ||||||||||
} | ||||||||||
|
||||||||||
else -> super.onOptionsItemSelected(item) | ||||||||||
} | ||||||||||
} | ||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,108 @@ | ||||||
package org.openobservatory.ooniprobe.activity.add_descriptor | ||||||
|
||||||
import androidx.lifecycle.MutableLiveData | ||||||
import androidx.lifecycle.ViewModel | ||||||
import androidx.lifecycle.viewmodel.ViewModelFactoryDsl | ||||||
import com.google.android.material.checkbox.MaterialCheckBox | ||||||
import com.google.android.material.checkbox.MaterialCheckBox.CheckedState | ||||||
import org.openobservatory.engine.OONIRunDescriptor | ||||||
import org.openobservatory.ooniprobe.activity.add_descriptor.adapter.GroupedItem | ||||||
import org.openobservatory.ooniprobe.common.LocaleUtils | ||||||
import org.openobservatory.ooniprobe.common.TestDescriptorManager | ||||||
import javax.inject.Inject | ||||||
|
||||||
/** | ||||||
* ViewModel for the AddDescriptorActivity. This class is responsible for preparing and managing the data for the AddDescriptorActivity. | ||||||
* It handles the communication of the Activity with the rest of the application (e.g. calling business logic classes). | ||||||
* | ||||||
* @property descriptorManager Instance of TestDescriptorManager which is responsible for managing the test descriptors. | ||||||
* @property selectedAllBtnStatus LiveData holding the state of the "Select All" button in the UI. | ||||||
* @property descriptor LiveData holding the OONIRunDescriptor object that the user is currently interacting with in the UI. | ||||||
*/ | ||||||
class AddDescriptorViewModel constructor( | ||||||
aanorbel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
var descriptorManager: TestDescriptorManager | ||||||
) : ViewModel() { | ||||||
@CheckedState | ||||||
val selectedAllBtnStatus: MutableLiveData<Int> = | ||||||
MutableLiveData(MaterialCheckBox.STATE_CHECKED) | ||||||
var descriptor: MutableLiveData<OONIRunDescriptor> = MutableLiveData() | ||||||
val finishActivity: MutableLiveData<Boolean> = MutableLiveData() | ||||||
|
||||||
/** | ||||||
* This method is called when the activity is created. | ||||||
* It sets the descriptor value of this ViewModel. | ||||||
* @param descriptor is the new descriptor | ||||||
*/ | ||||||
fun onDescriptorChanged(descriptor: OONIRunDescriptor) { | ||||||
aanorbel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
this.descriptor.value = descriptor | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is used to get the name of the descriptor. | ||||||
* Used by the UI during data binding. | ||||||
* @return the name of the descriptor. | ||||||
*/ | ||||||
fun getName(): String { | ||||||
return descriptor.value?.let { descriptor -> | ||||||
descriptor.nameIntl[LocaleUtils.sLocale.language] ?: descriptor.name | ||||||
} ?: "" | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is used to get the name of the descriptor. | ||||||
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.
Suggested change
|
||||||
* Used by the UI during data binding. | ||||||
* @return the name of the descriptor. | ||||||
*/ | ||||||
fun getDescription(): String { | ||||||
return descriptor.value?.let { descriptor -> | ||||||
descriptor.descriptionIntl[LocaleUtils.sLocale.language] ?: descriptor.description | ||||||
} ?: "" | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is used to get the short description of the descriptor. | ||||||
* Used by the UI during data binding. | ||||||
* @return the short description of the descriptor. | ||||||
*/ | ||||||
fun getShortDescription(): String { | ||||||
return descriptor.value?.let { descriptor -> | ||||||
descriptor.shortDescriptionIntl[LocaleUtils.sLocale.language] | ||||||
?: descriptor.shortDescription | ||||||
} ?: "" | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is used to set the state of the "Select All" button in the UI. | ||||||
* @param selectedStatus is the new state of the "Select All" button. | ||||||
*/ | ||||||
fun setSelectedAllBtnStatus(@CheckedState selectedStatus: Int) { | ||||||
selectedAllBtnStatus.postValue(selectedStatus) | ||||||
} | ||||||
|
||||||
|
||||||
/** | ||||||
* This method is called when the "Add Link" button is clicked. | ||||||
* It adds the descriptor to the descriptor manager and signals that the activity should finish. | ||||||
* @param selectedNettest is the list of selected nettests. | ||||||
* @param automatedUpdates is a boolean indicating whether automated updates should be enabled. | ||||||
*/ | ||||||
fun onAddButtonClicked(selectedNettest: List<GroupedItem>, automatedUpdates: Boolean) { | ||||||
descriptor.value?.let { descriptor -> | ||||||
descriptorManager.addDescriptor( | ||||||
descriptor = descriptor.apply { | ||||||
nettests = selectedNettest.filter { it.selected } | ||||||
}, | ||||||
automatedUpdates = automatedUpdates | ||||||
).also { | ||||||
finishActivity() | ||||||
} | ||||||
} ?: throw IllegalStateException("Descriptor is null") | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is used to signal that the activity should finish. | ||||||
*/ | ||||||
fun finishActivity() { | ||||||
finishActivity.value = true | ||||||
} | ||||||
} |
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.
Please, do not use
_
for package names. It seems to be against some Java coding style conventions set forth by Oracle and Google, as documented at https://stackoverflow.com/a/3179221.