-
-
Notifications
You must be signed in to change notification settings - Fork 45
Created base waiting loader and implemented in multiple connect screens #3378
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
Changes from all commits
30351f7
a047b54
dcb3df8
7aa3559
8ed5831
8e6fb7a
4ddf966
f1418b1
c4cb875
b68128e
096d1a5
b2c3bb8
3625dcd
c6e06d5
5e409d1
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,18 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:id="@+id/loadingContainer" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:contentDescription="@string/please_wait" | ||
| android:elevation="8dp" | ||
| android:gravity="center" | ||
| android:importantForAccessibility="noHideDescendants" | ||
| android:visibility="gone"> | ||
|
|
||
| <ProgressBar | ||
| android:id="@+id/progressBar" | ||
| style="?android:attr/progressBarStyleLarge" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" /> | ||
| </FrameLayout> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| package org.commcare.fragments.base | ||
|
|
||
| import android.os.Bundle | ||
| import android.view.LayoutInflater | ||
| import android.view.View | ||
| import android.view.ViewGroup | ||
| import android.widget.FrameLayout | ||
| import androidx.fragment.app.Fragment | ||
| import androidx.viewbinding.ViewBinding | ||
| import org.commcare.dalvik.databinding.LoadingBinding | ||
| import org.commcare.interfaces.base.BaseConnectView | ||
|
|
||
| abstract class BaseConnectFragment<B : ViewBinding> : | ||
| Fragment(), | ||
| BaseConnectView { | ||
| private var _binding: B? = null | ||
| val binding get() = _binding!! | ||
|
|
||
| private lateinit var loadingBinding: LoadingBinding | ||
| private lateinit var rootView: View | ||
|
|
||
| /** | ||
| * Implement this method in child fragments to inflate their specific binding. | ||
| */ | ||
| protected abstract fun inflateBinding( | ||
| inflater: LayoutInflater, | ||
| container: ViewGroup?, | ||
| ): B | ||
|
|
||
| override fun onCreateView( | ||
| inflater: LayoutInflater, | ||
| container: ViewGroup?, | ||
| savedInstanceState: Bundle?, | ||
| ): View { | ||
| // Inflate child fragment's main view | ||
| _binding = inflateBinding(inflater, container) | ||
| val mainView = binding.root | ||
|
|
||
| // Inflate loading layout | ||
| loadingBinding = LoadingBinding.inflate(inflater, container, false) | ||
| val loadingView = loadingBinding.root | ||
|
|
||
| // Create a parent container that holds both mainView and loadingView | ||
| val mergedLayout = | ||
| FrameLayout( | ||
| requireContext(), | ||
| ).apply { | ||
| layoutParams = | ||
| ViewGroup.LayoutParams( | ||
| ViewGroup.LayoutParams.MATCH_PARENT, | ||
| ViewGroup.LayoutParams.MATCH_PARENT, | ||
| ) | ||
| addView(mainView) | ||
| addView(loadingView) | ||
| } | ||
|
|
||
| // Hide loading by default | ||
| hideLoading() | ||
|
|
||
| rootView = mergedLayout | ||
| return rootView | ||
| } | ||
|
|
||
| override fun onDestroyView() { | ||
| super.onDestroyView() | ||
| _binding = null | ||
| // No need to nullify loadingBinding since it's lateinit — but safe practice to hide it | ||
| loadingBinding.root.visibility = View.GONE | ||
| } | ||
|
|
||
| override fun showLoading() { | ||
| loadingBinding.root.visibility = View.VISIBLE | ||
| } | ||
|
|
||
| override fun hideLoading() { | ||
| loadingBinding.root.visibility = View.GONE | ||
| } | ||
| } | ||
|
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. Optional: Since we are already touching a lot of Java code, what if we converted these files to Kotlin? This comment goes for all the other Java files in this PR as well.
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. @conroy-ricketts Good suggestions although don't think we have talked about converting the existing Java code to Kotling as part of functional PRs. Few thoughts on this -
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. @shubham1g5 Makes sense! I appreciate you sharing your thoughts on this. I really like the idea of making this a separate tech-debt effort because of the reasons you described. I also think it would be good to convert our existing code to Kotlin at some point in the future if we ever want to convert our UI to Jetpack Compose and reap the full benefit of that - I think that may be the best reason to pitch to the Product Management team. All good stuff to think about!
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. @OrangeAndGreen I'm really curious what your thoughts are here? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,6 @@ | |
| import android.view.ViewGroup; | ||
|
|
||
| import androidx.annotation.RequiresApi; | ||
| import androidx.core.content.ContextCompat; | ||
| import androidx.navigation.Navigation; | ||
| import androidx.recyclerview.widget.LinearLayoutManager; | ||
| import androidx.recyclerview.widget.RecyclerView; | ||
|
|
@@ -17,18 +16,18 @@ | |
| import org.commcare.android.database.connect.models.ConnectJobDeliveryRecord; | ||
| import org.commcare.android.database.connect.models.ConnectJobRecord; | ||
| import org.commcare.android.database.connect.models.ConnectPaymentUnitRecord; | ||
| import org.commcare.connect.PersonalIdManager; | ||
| import org.commcare.dalvik.R; | ||
| import org.commcare.dalvik.databinding.FragmentConnectProgressDeliveryBinding; | ||
| import org.commcare.views.connect.CircleProgressBar; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
|
|
||
| public class ConnectDeliveryProgressDeliveryFragment extends ConnectJobFragment { | ||
| private FragmentConnectProgressDeliveryBinding binding; | ||
| public class ConnectDeliveryProgressDeliveryFragment extends ConnectJobFragment<FragmentConnectProgressDeliveryBinding> { | ||
| private RecyclerView recyclerView; | ||
| private ConnectDeliveryProgressReportAdapter adapter; | ||
|
|
||
|
|
@@ -38,10 +37,9 @@ public static ConnectDeliveryProgressDeliveryFragment newInstance() { | |
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.N) | ||
| @Override | ||
| public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { | ||
| binding = FragmentConnectProgressDeliveryBinding.inflate(inflater, container, false); | ||
|
|
||
| binding.btnSync.setOnClickListener(view -> { | ||
| public @NotNull View onCreateView(@NotNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { | ||
| View view = super.onCreateView(inflater, container, savedInstanceState); | ||
| getBinding().btnSync.setOnClickListener(v -> { | ||
| ConnectDeliveryProgressFragment parentFragment = (ConnectDeliveryProgressFragment)getParentFragment(); | ||
| if (parentFragment != null) { | ||
| parentFragment.refresh(); | ||
|
|
@@ -51,17 +49,17 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa | |
|
|
||
| updateProgressSummary(); | ||
| populateDeliveryProgress(); | ||
| return binding.getRoot(); | ||
| return view; | ||
| } | ||
|
|
||
| public void updateProgressSummary() { | ||
| int completed = job.getCompletedVisits(); | ||
| int total = job.getMaxVisits(); | ||
| int percent = total > 0 ? (100 * completed / total) : 100; | ||
|
|
||
| CircleProgressBar progress = binding.connectProgressProgressBar; | ||
| CircleProgressBar progress = getBinding().connectProgressProgressBar; | ||
| progress.setProgress(percent); | ||
| binding.connectProgressProgressText.setText(String.format(Locale.getDefault(), "%d%%", percent)); | ||
| getBinding().connectProgressProgressText.setText(String.format(Locale.getDefault(), "%d%%", percent)); | ||
|
|
||
|
Comment on lines
58
to
63
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. Clamp computed values to safe ranges; avoid negative remaining and >100%. Guard against data inconsistencies and division rounding quirks. - int percent = total > 0 ? (100 * completed / total) : 100;
+ int percent = total > 0 ? (100 * completed / total) : 100;
+ if (percent > 100) percent = 100;
@@
- int remaining = unit.getMaxTotal() - approved;
+ int remaining = Math.max(0, unit.getMaxTotal() - approved);Also applies to: 80-83 🤖 Prompt for AI Agents |
||
| StringBuilder completedText = new StringBuilder( | ||
| getString(R.string.connect_progress_status, completed, total)); | ||
|
|
@@ -76,7 +74,7 @@ public void updateProgressSummary() { | |
| } | ||
| } | ||
|
|
||
| binding.connectProgressStatusText.setText(completedText.toString()); | ||
| getBinding().connectProgressStatusText.setText(completedText.toString()); | ||
| } | ||
|
|
||
| @RequiresApi(api = Build.VERSION_CODES.N) | ||
|
|
@@ -103,7 +101,7 @@ private void populateDeliveryProgress() { | |
| )); | ||
| } | ||
|
|
||
| recyclerView = binding.rvDeliveryProgressReport; | ||
| recyclerView = getBinding().rvDeliveryProgressReport; | ||
| recyclerView.setLayoutManager(new LinearLayoutManager(getContext())); | ||
| if (adapter == null) { | ||
| adapter = new ConnectDeliveryProgressReportAdapter( | ||
|
|
@@ -152,6 +150,11 @@ private HashMap<String, HashMap<String, Integer>> getStatusMap(ConnectJobRecord | |
| @Override | ||
| public void onResume() { | ||
| super.onResume(); | ||
| binding.getRoot().requestLayout(); | ||
| getBinding().getRoot().requestLayout(); | ||
| } | ||
|
|
||
| @Override | ||
| protected @NotNull FragmentConnectProgressDeliveryBinding inflateBinding(@NotNull LayoutInflater inflater, @Nullable ViewGroup container) { | ||
| return FragmentConnectProgressDeliveryBinding.inflate(inflater, container, false); | ||
| } | ||
| } | ||
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.
@jaypanchal-13 Please remove this