From a4ede2f7f71d69c0341517beefbc07b1324536ce Mon Sep 17 00:00:00 2001 From: Norbel Ambanumben Date: Sat, 2 Dec 2023 12:41:50 +0100 Subject: [PATCH] Updated PR based on review --- app/build.gradle | 1 - .../ooniprobe/activity/MainActivity.java | 3 +- .../ooniprobe/adapters/DashboardAdapter.kt | 18 +- .../ooniprobe/fragment/ProgressFragment.kt | 339 +++++++++--------- gradle/libs.versions.toml | 1 - 5 files changed, 184 insertions(+), 178 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 6aa91d6f4..1cca3cb7d 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -103,7 +103,6 @@ dependencies { // AndroidX implementation libs.androidx.appcompat - //implementation libs.androidx.core implementation libs.androidx.constraintlayout implementation libs.androidx.lifecycle.process implementation libs.androidx.preference diff --git a/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java b/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java index b3e46ed6e..e0f79d624 100644 --- a/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java +++ b/app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java @@ -93,7 +93,8 @@ protected void onCreate(@Nullable Bundle savedInstanceState) { return false; } }); - // TODO(aanorbel): Fix change in state(theme change from notification) changing the selected item. + /* TODO(aanorbel): Fix change in state(theme change from notification) changes the selected item. + The proper fix would be to track the selected item as well as other properties in a `ViewModel`. */ binding.bottomNavigation.setSelectedItemId(getIntent().getIntExtra(RES_ITEM, R.id.dashboard)); /* Check if we are restoring the activity from a saved state first. * If we have a message to show, show it as a snackbar. diff --git a/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt b/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt index 4c7efef84..93d0d6679 100644 --- a/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt +++ b/app/src/main/java/org/openobservatory/ooniprobe/adapters/DashboardAdapter.kt @@ -48,9 +48,9 @@ class DashboardAdapter( val item = items[position] when (holder.itemViewType) { VIEW_TYPE_TITLE -> { - val separator = holder as CardGroupTitleViewHolder - separator.binding.root.text = item as String - } + val separator = holder as CardGroupTitleViewHolder + separator.binding.root.text = item as String + } VIEW_TYPE_CARD -> { val cardHolder = holder as CardViewHolder @@ -72,7 +72,10 @@ class DashboardAdapter( holder.binding.apply { title.setTextColor(resources.getColor(R.color.disabled_test_text)) desc.setTextColor(resources.getColor(R.color.disabled_test_text)) - icon.setColorFilter(resources.getColor(R.color.disabled_test_text), PorterDuff.Mode.SRC_IN) + icon.setColorFilter( + resources.getColor(R.color.disabled_test_text), + PorterDuff.Mode.SRC_IN + ) } } else { holder.itemView.setOnClickListener(onClickListener) @@ -94,13 +97,14 @@ class DashboardAdapter( } /** - * ViewHolder for dashboard item group + * ViewHolder for a dashboard item group. * @param binding */ - class CardGroupTitleViewHolder(var binding: ItemSeperatorBinding) : RecyclerView.ViewHolder(binding.root) + class CardGroupTitleViewHolder(var binding: ItemSeperatorBinding) : + RecyclerView.ViewHolder(binding.root) /** - * ViewHolder for dashboard item + * ViewHolder for dashboard item. * @param binding */ class CardViewHolder(var binding: ItemTestsuiteBinding) : RecyclerView.ViewHolder(binding.root) diff --git a/app/src/main/java/org/openobservatory/ooniprobe/fragment/ProgressFragment.kt b/app/src/main/java/org/openobservatory/ooniprobe/fragment/ProgressFragment.kt index 347ff72fe..86132021a 100644 --- a/app/src/main/java/org/openobservatory/ooniprobe/fragment/ProgressFragment.kt +++ b/app/src/main/java/org/openobservatory/ooniprobe/fragment/ProgressFragment.kt @@ -28,172 +28,175 @@ import javax.inject.Inject * Monitors and displays progress of [RunTestService]. */ class ProgressFragment : Fragment() { - private lateinit var receiver: TestRunBroadRequestReceiver - private lateinit var biding: FragmentProgressBinding - - @Inject - lateinit var preferenceManager: PreferenceManager - - @Inject - lateinit var testProgressRepository: TestProgressRepository - - override fun onCreateView( - inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? - ): View { - biding = FragmentProgressBinding.inflate(inflater, container, false) - (requireActivity().application as Application).fragmentComponent.inject(this) - biding.root.setOnClickListener { _: View? -> - val intent = Intent(context, RunningActivity::class.java) - ActivityCompat.startActivity(requireContext(), intent, null) - } - testProgressRepository.progress.observe(viewLifecycleOwner) { progressValue: Int? -> - if (progressValue != null) { - biding.progress.progress = progressValue - } - } - return biding.root - } - - override fun onResume() { - super.onResume() - val filter = IntentFilter("org.openobservatory.ooniprobe.activity.RunningActivity") - receiver = TestRunBroadRequestReceiver( - preferenceManager, TestRunnerEventListener(), testProgressRepository - ) - // NOTE: Simple update to ContextCompat#registerReceiver not possible at the moment. - LocalBroadcastManager.getInstance(requireContext()).registerReceiver(receiver, filter) - bindTestService() - } - - fun bindTestService() { - if ((requireActivity().application as Application).isTestRunning) { - requireContext().bindService( - Intent(requireContext(), RunTestService::class.java), - receiver, - Context.BIND_AUTO_CREATE - ) - biding.progressLayout.visibility = View.VISIBLE - } else { - biding.progressLayout.visibility = View.GONE - } - } - - private fun updateUI(service: RunTestService?) { - if ((requireActivity().application as Application).isTestRunning) { - val progressLevel = testProgressRepository.progress.value - when { - progressLevel != null -> { - biding.progress.progress = progressLevel - } - else -> { - biding.progress.isIndeterminate = true - } - } - biding.testImage.setImageDrawable(null) - biding.testImage.visibility = View.GONE - - service?.task?.let { task -> - task.currentSuite?.let { - biding.progress.max = service.task.getMax(preferenceManager) - } - task.currentTest?.let { currentTest -> - when { - /** - * If the test has no icon, use the suite icon. - * Currently not used since the most suites have a larger than desired icon. - */ - /*currentTest.iconResId == 0 -> { - biding.testImage.apply { - visibility = View.VISIBLE - setImageDrawable( - ContextCompat.getDrawable( - requireContext(), - task.currentSuite.icon - ) - ) - } - }*/ - /** - * If the test has an icon, use it. - */ - currentTest.iconResId != 0 -> { - biding.testImage.apply { - visibility = View.VISIBLE - setImageDrawable( - ContextCompat.getDrawable( - requireContext(), - task.currentTest.iconResId - ) - ) - } - } - - else -> { - biding.testImage.visibility = View.GONE - } - } - biding.name.text = when (task.currentSuite is ExperimentalSuite) { - true -> SpannableStringBuilder().bold { append(currentTest.name) } - false -> SpannableStringBuilder().bold { append(getString(currentTest.labelResId)) } - }.append(" ") - .append( - getString(R.string.Dashboard_Running_Running) - .replace(":", "").toLowerCase() - ) - - } - } - } - } - - override fun onPause() { - super.onPause() - if (receiver.isBound) { - requireContext().unbindService(receiver) - receiver.isBound = false - } - LocalBroadcastManager.getInstance(requireContext()).unregisterReceiver(receiver) - } - - override fun onDestroy() { - super.onDestroy() - LocalBroadcastManager.getInstance(requireContext()).unregisterReceiver(receiver) - } - - private inner class TestRunnerEventListener : TestRunBroadRequestReceiver.EventListener { - override fun onStart(service: RunTestService) = updateUI(service) - - override fun onRun(value: String) { - biding.name.text = value - } - - override fun onProgress(state: Int, eta: Double) { - updateUI(receiver.service) - - biding.progress.apply { - isIndeterminate = false - progress = state - } - } - - override fun onLog(value: String) {/* nothing */ - } - - override fun onError(value: String) {/* nothing */ - } - - override fun onUrl() { - biding.progress.isIndeterminate = false - } - - override fun onInterrupt() { - biding.testImage.setImageDrawable(null) - biding.testImage.visibility = View.GONE - biding.name.text = getString(R.string.Dashboard_Running_Stopping_Title) - } - - override fun onEnd(context: Context) { - biding.progressLayout.visibility = View.GONE - } - } + private lateinit var receiver: TestRunBroadRequestReceiver + private lateinit var biding: FragmentProgressBinding + + @Inject + lateinit var preferenceManager: PreferenceManager + + @Inject + lateinit var testProgressRepository: TestProgressRepository + + override fun onCreateView( + inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? + ): View { + biding = FragmentProgressBinding.inflate(inflater, container, false) + (requireActivity().application as Application).fragmentComponent.inject(this) + biding.root.setOnClickListener { _: View? -> + val intent = Intent(context, RunningActivity::class.java) + ActivityCompat.startActivity(requireContext(), intent, null) + } + testProgressRepository.progress.observe(viewLifecycleOwner) { progressValue: Int? -> + if (progressValue != null) { + biding.progress.progress = progressValue + } + } + return biding.root + } + + override fun onResume() { + super.onResume() + val filter = IntentFilter("org.openobservatory.ooniprobe.activity.RunningActivity") + receiver = TestRunBroadRequestReceiver( + preferenceManager, TestRunnerEventListener(), testProgressRepository + ) + // NOTE: Simple update to ContextCompat#registerReceiver not possible at the moment. + LocalBroadcastManager.getInstance(requireContext()).registerReceiver(receiver, filter) + bindTestService() + } + + fun bindTestService() { + if ((requireActivity().application as Application).isTestRunning) { + requireContext().bindService( + Intent(requireContext(), RunTestService::class.java), + receiver, + Context.BIND_AUTO_CREATE + ) + biding.progressLayout.visibility = View.VISIBLE + } else { + biding.progressLayout.visibility = View.GONE + } + } + + private fun updateUI(service: RunTestService?) { + if ((requireActivity().application as Application).isTestRunning) { + val progressLevel = testProgressRepository.progress.value + when { + progressLevel != null -> { + biding.progress.progress = progressLevel + } + + else -> { + biding.progress.isIndeterminate = true + } + } + biding.testImage.setImageDrawable(null) + biding.testImage.visibility = View.GONE + + service?.task?.let { task -> + task.currentSuite?.let { + biding.progress.max = service.task.getMax(preferenceManager) + } + task.currentTest?.let { currentTest -> + when { + /** + * If the test has no icon, use the suite icon. + * Currently not used since the most suites have a larger than desired icon. + */ + /*currentTest.iconResId == 0 -> { + biding.testImage.apply { + visibility = View.VISIBLE + setImageDrawable( + ContextCompat.getDrawable( + requireContext(), + task.currentSuite.icon + ) + ) + } + }*/ + /** + * If the test has an icon, use it. + */ + currentTest.iconResId != 0 -> { + biding.testImage.apply { + visibility = View.VISIBLE + setImageDrawable( + ContextCompat.getDrawable( + requireContext(), + task.currentTest.iconResId + ) + ) + } + } + + else -> { + biding.testImage.visibility = View.GONE + } + } + biding.name.text = when (task.currentSuite is ExperimentalSuite) { + true -> SpannableStringBuilder().bold { append(currentTest.name) } + false -> SpannableStringBuilder().bold { append(getString(currentTest.labelResId)) } + }.append(" ") + .append( + getString(R.string.Dashboard_Running_Running) + .replace(":", "").toLowerCase() + ) + + } + } + } + } + + override fun onPause() { + super.onPause() + if (receiver.isBound) { + requireContext().unbindService(receiver) + receiver.isBound = false + } + LocalBroadcastManager.getInstance(requireContext()).unregisterReceiver(receiver) + } + + override fun onDestroy() { + super.onDestroy() + LocalBroadcastManager.getInstance(requireContext()).unregisterReceiver(receiver) + } + + private inner class TestRunnerEventListener : TestRunBroadRequestReceiver.EventListener { + override fun onStart(service: RunTestService) = updateUI(service) + + override fun onRun(value: String) { + biding.name.text = value + } + + override fun onProgress(state: Int, eta: Double) { + updateUI(receiver.service) + + biding.progress.apply { + isIndeterminate = false + progress = state + } + } + + override fun onLog(value: String) { + /* nothing */ + } + + override fun onError(value: String) { + /* nothing */ + } + + override fun onUrl() { + biding.progress.isIndeterminate = false + } + + override fun onInterrupt() { + biding.testImage.setImageDrawable(null) + biding.testImage.visibility = View.GONE + biding.name.text = getString(R.string.Dashboard_Running_Stopping_Title) + } + + override fun onEnd(context: Context) { + biding.progressLayout.visibility = View.GONE + } + } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a21600a5c..135bcef68 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -78,7 +78,6 @@ dbflow-lib = { module = "com.github.Raizlabs.DBFlow:dbflow", version.ref = "dbfl dbflow-core = { module = "com.github.Raizlabs.DBFlow:dbflow-core", version.ref = "dbflow" } dbflow-processor = { module = "com.github.Raizlabs.DBFlow:dbflow-processor", version.ref = "dbflow" } -# androidx-core = { group = "androidx.core", name = "core", version.ref = "androidxCore" } androidx-test-core = { module = "androidx.test:core", version.ref = "androidxCore" } androidx-rules = { module = "androidx.test:rules", version.ref = "androidxCore" } androidx-runner = { module = "androidx.test:runner", version.ref = "androidxRunner" }