Skip to content

feat: replace toast with custom success dialog - #258

Merged
dz4va merged 2 commits into
developfrom
AJ-658
May 27, 2026
Merged

feat: replace toast with custom success dialog#258
dz4va merged 2 commits into
developfrom
AJ-658

Conversation

@amitsinghsutara

@amitsinghsutara amitsinghsutara commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Changes

  • replace toast with custom success dialog

Ref: AJ-658

Summary by CodeRabbit

  • New Features

    • Added success dialog that displays after ID enrollment and auto-dismisses after approximately 2 seconds
    • Language selection flow now triggers after enrollment completion if no language preference is set
  • Chores

    • Updated app version to 2.34.3

Review Change Stack

dz4va and others added 2 commits May 27, 2026 00:42
- Persist confirmed study_user_id after successful study enrollment
- Store the study ID alongside the existing pseudoId using a dedicated preference key
- Append study_user_id only to app_launch Firebase event parameters when available
- Leave unenrolled app_launch events unchanged by omitting study_user_id
- Add unit coverage for app_launch events with and without a persisted study_user_id
- Restore the existing language_selected analytics test invocation so the test class passes
@amitsinghsutara
amitsinghsutara requested a review from dz4va May 27, 2026 16:23
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR enhances analytics tracking by storing a study user ID during ID confirmation and enriches app_launch events with that ID, while updating the confirmation UX with a new success dialog that auto-dismisses and displays celebratory feedback before showing the language selection.

Changes

Study User ID Analytics and Enrollment Success

Layer / File(s) Summary
Analytics metadata and event enrichment
app/src/main/java/org/curiouslearning/container/firebase/AnalyticsUtils.java
Introduces APP_CACHED_PREFS and STUDY_USER_ID constants for analytics tracking; adds a helper that reads the stored study user ID and enriches app_launch events with it in the Firebase Bundle; wires the helper into logEvent before analytics dispatch.
ID confirmation and success dialog
app/src/main/java/org/curiouslearning/container/MainActivity.java, app/src/main/res/layout/dialog_enrollment_success.xml
ID confirmation now stores the confirmed ID to the STUDY_USER_ID preference key alongside pseudoId; removes the post-confirmation toast and shows a new enrollment success dialog instead; adds showSuccessDialog helper that displays the success layout, auto-dismisses after 2 seconds, and triggers the language popup on dismissal; new layout resource displays a centered 🎉 emoji.
Analytics and success dialog test coverage
app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java
Test fixture now manages cached preferences; testLogEvent_appLaunch() seeds the preference and verifies study_user_id is included in the event Bundle; new testLogEvent_appLaunchWithoutStudyUserId() confirms the field is absent when not stored; language event test is updated with the new signature.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • curiouslearning/CRcontainer#254: Introduces the ID confirmation UI and confirm handling; this PR extends that flow to write study_user_id to analytics and display a success dialog.

Suggested reviewers

  • dz4va
  • Rajesh1041

Poem

A rabbit hops through metrics true, 🐰
Storing IDs in prefs anew,
Success dialogs dance and cheer,
Analytics enriched, crystal clear! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Tests & Lint & Coverage ⚠️ Warning Tests are added; linting runs automatically. However, no 70% test coverage threshold is enforced—Jacoco is configured but lacks minimum coverage requirement in build.gradle. Configure Jacoco coverage threshold in app/build.gradle to enforce minimum 70% coverage and fail the build if not met.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly reflects the main change in the changeset: replacing a toast notification with a custom success dialog, as evidenced by MainActivity.java modifications and the new dialog_enrollment_success.xml layout.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch AJ-658

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/main/java/org/curiouslearning/container/MainActivity.java`:
- Around line 470-506: The showSuccessDialog method currently calls
successDialog.show() and schedules dismissal without checking Activity state;
before creating or showing the Dialog (inside runOnUiThread), guard with
Activity lifecycle checks (isFinishing() and, on API >=17, isDestroyed()) to
avoid showing or scheduling UI when the Activity is finishing/destroyed; apply
the same guard before calling successDialog.show(), before postDelayed
scheduling, and in the OnDismissListener to ensure onDismissAction.run() is only
invoked when the Activity is valid.

In `@app/src/main/res/layout/dialog_enrollment_success.xml`:
- Around line 17-25: The TextView in dialog_enrollment_success.xml is using a
hardcoded emoji ("🎉") in android:text which is not accessible; replace the
hardcoded text with a string resource (e.g., add <string
name="enrollment_success_message">Enrollment successful</string> to
res/values/strings.xml) and set the TextView's android:text to that resource
(android:text="`@string/enrollment_success_message`"); ensure the TextView also
provides an accessible label for TalkBack (e.g., via android:contentDescription
referencing the same string) so screen readers receive meaningful feedback.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6d1ea314-6d75-4eca-a326-1c63792589a5

📥 Commits

Reviewing files that changed from the base of the PR and between 79d5323 and 9b08db1.

📒 Files selected for processing (5)
  • app/build.gradle
  • app/src/main/java/org/curiouslearning/container/MainActivity.java
  • app/src/main/java/org/curiouslearning/container/firebase/AnalyticsUtils.java
  • app/src/main/res/layout/dialog_enrollment_success.xml
  • app/src/test/java/org/curiouslearning/container/AnalyticsUtilsCustomEventsTest.java

Comment on lines +470 to +506
private void showSuccessDialog(Runnable onDismissAction) {
runOnUiThread(() -> {
try {
final Dialog successDialog = new Dialog(this);
successDialog.setContentView(R.layout.dialog_enrollment_success);
successDialog.setCanceledOnTouchOutside(true);
successDialog.setOnDismissListener(dialog -> {
if (onDismissAction != null) {
onDismissAction.run();
}
});
if (successDialog.getWindow() != null) {
successDialog.getWindow().setBackgroundDrawable(new android.graphics.drawable.ColorDrawable(android.graphics.Color.TRANSPARENT));
}

View container = successDialog.findViewById(R.id.success_container);
if (container != null) {
container.setOnClickListener(v -> {
if (successDialog.isShowing()) {
successDialog.dismiss();
}
});
}

successDialog.show();

new Handler(Looper.getMainLooper()).postDelayed(() -> {
if (successDialog.isShowing()) {
successDialog.dismiss();
}
}, 2000);

} catch (Exception e) {
Log.e(TAG, "showSuccessDialog: Failed to show success dialog", e);
}
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add an Activity lifecycle guard before showing the success dialog.

At Line 494, successDialog.show() can still run while the Activity is finishing/destroyed. That path is caught, but it can silently skip the intended post-confirmation UX flow.

Proposed fix
 private void showSuccessDialog(Runnable onDismissAction) {
     runOnUiThread(() -> {
         try {
+            if (isFinishing() || isDestroyed()) {
+                Log.w(TAG, "showSuccessDialog: Activity is finishing or destroyed, not showing dialog.");
+                return;
+            }
             final Dialog successDialog = new Dialog(this);
             successDialog.setContentView(R.layout.dialog_enrollment_success);
             successDialog.setCanceledOnTouchOutside(true);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/org/curiouslearning/container/MainActivity.java` around
lines 470 - 506, The showSuccessDialog method currently calls
successDialog.show() and schedules dismissal without checking Activity state;
before creating or showing the Dialog (inside runOnUiThread), guard with
Activity lifecycle checks (isFinishing() and, on API >=17, isDestroyed()) to
avoid showing or scheduling UI when the Activity is finishing/destroyed; apply
the same guard before calling successDialog.show(), before postDelayed
scheduling, and in the OnDismissListener to ensure onDismissAction.run() is only
invoked when the Activity is valid.

Comment on lines +17 to +25
<TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="🎉"
android:textSize="80sp"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent" />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid emoji-only hardcoded text for success messaging.

At Line 20, the dialog uses hardcoded emoji text only. Please move text to a string resource and provide accessible success text so TalkBack users get meaningful feedback.

Proposed fix
-        <TextView
+        <TextView
+            android:id="@+id/success_message"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
-            android:text="🎉"
+            android:text="`@string/enrollment_success_message`"
+            android:contentDescription="`@string/enrollment_success_message`"
             android:textSize="80sp"
             app:layout_constraintTop_toTopOf="parent"
             app:layout_constraintBottom_toBottomOf="parent"
             app:layout_constraintStart_toStartOf="parent"
             app:layout_constraintEnd_toEndOf="parent" />
<!-- res/values/strings.xml -->
<string name="enrollment_success_message">Enrollment successful</string>
📝 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
<TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="🎉"
android:textSize="80sp"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent" />
<TextView
android:id="@+id/success_message"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="`@string/enrollment_success_message`"
android:contentDescription="`@string/enrollment_success_message`"
android:textSize="80sp"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent" />
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/res/layout/dialog_enrollment_success.xml` around lines 17 - 25,
The TextView in dialog_enrollment_success.xml is using a hardcoded emoji ("🎉")
in android:text which is not accessible; replace the hardcoded text with a
string resource (e.g., add <string name="enrollment_success_message">Enrollment
successful</string> to res/values/strings.xml) and set the TextView's
android:text to that resource
(android:text="`@string/enrollment_success_message`"); ensure the TextView also
provides an accessible label for TalkBack (e.g., via android:contentDescription
referencing the same string) so screen readers receive meaningful feedback.

@dz4va
dz4va merged commit e6c54bd into develop May 27, 2026
1 check passed
@amitsinghsutara
amitsinghsutara deleted the AJ-658 branch June 18, 2026 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants