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

CCCT-567 || Home Screen UI Fixes #2917

Merged
merged 6 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
93 changes: 39 additions & 54 deletions app/src/org/commcare/activities/HomeButtons.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
import android.view.View;
import android.widget.Toast;

import org.commcare.connect.ConnectManager;
import org.commcare.adapters.HomeCardDisplayData;
import org.commcare.adapters.SquareButtonViewHolder;
import org.commcare.connect.ConnectManager;
import org.commcare.dalvik.R;
import org.commcare.google.services.analytics.AnalyticsParamValue;
import org.commcare.google.services.analytics.FirebaseAnalyticsUtil;
Expand All @@ -18,6 +18,8 @@
import org.commcare.utils.SyncDetailCalculations;
import org.javarosa.core.services.locale.Localization;

import java.util.ArrayList;
import java.util.List;
import java.util.Vector;

/**
Expand All @@ -27,8 +29,8 @@
*/
public class HomeButtons {

private final static String[] buttonNames =
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies, I should have provided more guidance here. Hiding the button can be accomplished more easily and without having to make additional modifications to this code. Instead, we just need to enhance StandardHomeActivityUIController.getHiddenButtons so it correctly indicates when to hide the Job Status button. The list of button names returned by that function is passed through to HomeButtons so buttons can be shown or hidden as indicated.

new String[]{"start", "training", "saved", "incomplete", "connect", "sync", "report", "logout"};
private static String[] buttonNames =
new String[]{};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor button management to improve maintainability

The current implementation relies on the order of the buttonNames array aligning with the buttons list, which can be error-prone and difficult to maintain. This tight coupling may lead to issues if buttons are added, removed, or reordered in the future.

Consider using a single data structure, such as a list of custom objects or a map, that encapsulates both the button identifiers and their corresponding HomeCardDisplayData. This approach can eliminate the need for parallel arrays and reduce the risk of indexing errors.

Also applies to: 48-50, 52-65


/**
* Note: The order in which home cards are returned by this method should be consistent with
Expand All @@ -37,62 +39,45 @@ public class HomeButtons {
public static HomeCardDisplayData[] buildButtonData(StandardHomeActivity activity,
Vector<String> buttonsToHide,
boolean isDemoUser) {
String syncKey, homeMessageKey, logoutMessageKey;
if (!isDemoUser) {
homeMessageKey = "home.start";
syncKey = "home.sync";
logoutMessageKey = "home.logout";
} else {
syncKey = "home.sync.demo";
homeMessageKey = "home.start.demo";
logoutMessageKey = "home.logout.demo";
String homeMessageKey = isDemoUser ? "home.start.demo" : "home.start";
String syncKey = isDemoUser ? "home.sync.demo" : "home.sync";
String logoutMessageKey = isDemoUser ? "home.logout.demo" : "home.logout";

boolean hideJobStatus = ConnectManager.shouldShowJobStatus(activity);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential logical error in hideJobStatus assignment

The variable hideJobStatus may be incorrectly assigned. The method ConnectManager.shouldShowJobStatus(activity) likely returns true when the job status should be shown, but assigning it to hideJobStatus implies the opposite.

Consider adjusting the assignment to:

- boolean hideJobStatus = ConnectManager.shouldShowJobStatus(activity);
+ boolean hideJobStatus = !ConnectManager.shouldShowJobStatus(activity);

This ensures that hideJobStatus accurately represents whether the job status should be hidden.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
boolean hideJobStatus = ConnectManager.shouldShowJobStatus(activity);
boolean hideJobStatus = !ConnectManager.shouldShowJobStatus(activity);


buttonNames = hideJobStatus ?
new String[]{"start", "training", "saved", "incomplete", "sync", "report", "logout"} :
new String[]{"start", "training", "saved", "incomplete", "connect", "sync", "report", "logout"};

List<HomeCardDisplayData> buttons = new ArrayList<>();

buttons.add(createStaticButton(Localization.get(homeMessageKey), R.color.white, R.drawable.start, R.color.cc_attention_positive_color, getStartButtonListener(activity)));
buttons.add(createStaticButton(Localization.get("training.root.title"), R.color.white, R.drawable.home_training, R.color.cc_dark_cool_accent_color, getTrainingButtonListener(activity)));
buttons.add(createStaticButton(Localization.get("home.forms.saved"), R.color.white, R.drawable.home_saved, R.color.cc_light_cool_accent_color, getViewOldFormsListener(activity)));
buttons.add(createDynamicButton(Localization.get("home.forms.incomplete"), R.color.white, R.drawable.home_incomplete, R.color.solid_dark_orange, getIncompleteButtonListener(activity), null, getIncompleteButtonTextSetter(activity)));
if (!hideJobStatus) {
buttons.add(createStaticButton(Localization.get("home.connect"), R.color.white, R.drawable.baseline_save_24, R.color.orange_500, getConnectButtonListener(activity)));
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect logic in condition for adding the "connect" button

The condition if (!hideJobStatus) may be affected by the assignment issue of hideJobStatus. If hideJobStatus is incorrectly set, this condition could lead to the "connect" button being displayed or hidden unintentionally.

After correcting the hideJobStatus assignment, ensure that this condition correctly reflects the intended logic for displaying the "connect" button.

buttons.add(createNotificationButton(Localization.get(syncKey), R.color.white, R.color.white, R.drawable.sync, R.color.cc_brand_color, R.color.cc_brand_text, getSyncButtonListener(activity), getSyncButtonSubTextListener(activity), getSyncButtonTextSetter(activity)));
buttons.add(createStaticButton(Localization.get("home.report"), R.color.white, R.drawable.home_report, R.color.cc_attention_negative_color, getReportButtonListener(activity)));
buttons.add(createNotificationButton(Localization.get(logoutMessageKey), R.color.white, R.color.white, R.drawable.logout, R.color.cc_neutral_color, R.color.cc_neutral_text, getLogoutButtonListener(activity), null, getLogoutButtonTextSetter(activity)));

HomeCardDisplayData[] allButtons = new HomeCardDisplayData[]{
HomeCardDisplayData.homeCardDataWithStaticText(Localization.get(homeMessageKey),
R.color.white,
R.drawable.start,
R.color.cc_attention_positive_color,
getStartButtonListener(activity)),
HomeCardDisplayData.homeCardDataWithStaticText(Localization.get("training.root.title"), R.color.white,
R.drawable.home_training, R.color.cc_dark_cool_accent_color,
getTrainingButtonListener(activity)),
HomeCardDisplayData.homeCardDataWithStaticText(Localization.get("home.forms.saved"),
R.color.white,
R.drawable.home_saved,
R.color.cc_light_cool_accent_color,
getViewOldFormsListener(activity)),
HomeCardDisplayData.homeCardDataWithDynamicText(Localization.get("home.forms.incomplete"), R.color.white,
R.drawable.home_incomplete,
R.color.solid_dark_orange,
getIncompleteButtonListener(activity),
null,
getIncompleteButtonTextSetter(activity)),
HomeCardDisplayData.homeCardDataWithStaticText(Localization.get("home.connect"), R.color.white,
R.drawable.baseline_save_24, R.color.orange_500,
getConnectButtonListener(activity)),
HomeCardDisplayData.homeCardDataWithNotification(Localization.get(syncKey), R.color.white,
R.color.white,
R.drawable.sync,
R.color.cc_brand_color,
R.color.cc_brand_text,
getSyncButtonListener(activity),
getSyncButtonSubTextListener(activity),
getSyncButtonTextSetter(activity)),
HomeCardDisplayData.homeCardDataWithStaticText(Localization.get("home.report"), R.color.white,
R.drawable.home_report, R.color.cc_attention_negative_color,
getReportButtonListener(activity)),
HomeCardDisplayData.homeCardDataWithNotification(Localization.get(logoutMessageKey), R.color.white,
R.color.white,
R.drawable.logout, R.color.cc_neutral_color, R.color.cc_neutral_text,
getLogoutButtonListener(activity),
null,
getLogoutButtonTextSetter(activity)),
};
return getVisibleButtons(buttons.toArray(new HomeCardDisplayData[0]), buttonsToHide);
}

private static HomeCardDisplayData createStaticButton(String text, int textColor, int imageResource, int bgColor, View.OnClickListener listener) {
return HomeCardDisplayData.homeCardDataWithStaticText(text, textColor, imageResource, bgColor, listener);
}

return getVisibleButtons(allButtons, buttonsToHide);
private static HomeCardDisplayData createDynamicButton(String text, int textColor, int imageResource, int bgColor, View.OnClickListener listener, View.OnClickListener subTextListener, HomeButtons.TextSetter textSetter) {
return HomeCardDisplayData.homeCardDataWithDynamicText(text, textColor, imageResource, bgColor, listener, subTextListener, textSetter);
}

private static HomeCardDisplayData createNotificationButton(String text, int textColor, int subTextColor, int imageResource, int bgColor, int subTextBgColor, View.OnClickListener listener, View.OnClickListener subTextListener, HomeButtons.TextSetter textSetter) {
return HomeCardDisplayData.homeCardDataWithNotification(text, textColor, subTextColor, imageResource, bgColor, subTextBgColor, listener, subTextListener, textSetter);
}


private static HomeCardDisplayData[] getVisibleButtons(HomeCardDisplayData[] allButtons,
Vector<String> buttonsToHide) {
int visibleButtonCount = buttonNames.length - buttonsToHide.size();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package org.commcare.activities;

import static org.commcare.android.database.connect.models.ConnectJobRecord.STATUS_DELIVERING;

import android.annotation.SuppressLint;
import android.content.Context;
import android.view.View;
import android.view.ViewTreeObserver;
import android.widget.ProgressBar;
import android.widget.TextView;

import androidx.cardview.widget.CardView;
Expand Down Expand Up @@ -35,7 +36,6 @@
import java.util.Date;
import java.util.Hashtable;
import java.util.List;
import java.util.Locale;
import java.util.Vector;

/**
Expand Down Expand Up @@ -78,8 +78,7 @@ private void updateJobTileDetails() {
boolean show = record != null && !record.getIsLearning();

viewJobCard.setVisibility(show ? View.VISIBLE : View.GONE);

if(show) {
if (show) {
ConnectBoldTextView tvJobTitle = viewJobCard.findViewById(R.id.tv_job_title);
ConnectMediumTextView tvViewMore = viewJobCard.findViewById(R.id.tv_view_more);
ConnectMediumTextView tvJobDiscrepation = viewJobCard.findViewById(R.id.tv_job_discrepation);
Expand All @@ -98,7 +97,7 @@ private void updateJobTileDetails() {
boolean showHours = workingHours != null;
tv_job_time.setVisibility(showHours ? View.VISIBLE : View.GONE);
hoursTitle.setVisibility(showHours ? View.VISIBLE : View.GONE);
if(showHours) {
if (showHours) {
tv_job_time.setText(workingHours);
}

Expand All @@ -111,7 +110,7 @@ private void updateOpportunityMessage() {
String appId = CommCareApplication.instance().getCurrentApp().getUniqueId();
ConnectAppRecord record = ConnectManager.getAppRecord(activity, appId);
boolean show = record != null && !record.getIsLearning();
if(show) {
if (show) {
ConnectJobRecord job = ConnectManager.getActiveJob();
if (job.isFinished()) {
warningText = activity.getString(R.string.connect_progress_warning_ended);
Expand Down Expand Up @@ -147,14 +146,14 @@ private void updateOpportunityMessage() {
}
}

if(totalMaxes.size() > 0 || dailyMaxes.size() > 0) {
if (totalMaxes.size() > 0 || dailyMaxes.size() > 0) {
warningText = "";
if(totalMaxes.size() > 0) {
if (totalMaxes.size() > 0) {
String maxes = String.join(", ", totalMaxes);
warningText = activity.getString(R.string.connect_progress_warning_max_reached_multi, maxes);
}

if(dailyMaxes.size() > 0) {
if (dailyMaxes.size() > 0) {
String maxes = String.join(", ", dailyMaxes);
warningText += activity.getString(R.string.connect_progress_warning_daily_max_reached_multi, maxes);
}
Expand All @@ -169,7 +168,7 @@ private void updateOpportunityMessage() {
}

connectMessageCard.setVisibility(warningText == null ? View.GONE : View.VISIBLE);
if(warningText != null) {
if (warningText != null) {
TextView tv = connectMessageCard.findViewById(R.id.tvConnectMessage);
tv.setText(warningText);
}
Expand All @@ -195,11 +194,15 @@ public void updateConnectProgress() {
RecyclerView recyclerView = viewJobCard.findViewById(R.id.rdDeliveryTypeList);
ConnectJobRecord job = ConnectManager.getActiveJob();

if (job.getStatus() == STATUS_DELIVERING && job.isFinished()) {
recyclerView.setVisibility(View.GONE);
}

updateOpportunityMessage();

deliveryPaymentInfoList.clear();

if(job != null) {
if (job != null) {
//Note: Only showing a single daily progress bar for now
//Adding more entries to the list would show multiple progress bars
//(i.e. one for each payment type)
Expand Down
Loading