ADFA-2685: Delete 2nd onboarding slide, move privacy disclosure to slide 3#1331
Merged
Merged
Conversation
…ide 3 Streamlines onboarding from three slides to two by dropping the informational "Next page: permissions" slide. The Privacy & analytics disclosure dialog — previously triggered on slide 2's onSlideSelected — now fires on the Permissions slide via the same listener hook, preserving the once-per-install preference key so existing users aren't re-prompted. Removes the now-unused fragment, layout, adapter, model, test helper, and the five permissions_info_* strings (en + id).
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 Walkthrough
Risks & best-practice considerations:
WalkthroughThe PR removes the dedicated permissions-info onboarding slide and consolidates its privacy disclosure functionality into the permissions slide. PermissionsFragment now displays the privacy dialog on first selection, with options to accept or view the privacy policy. Onboarding activities, test scenarios, and string resources are updated accordingly to skip the removed slide. ChangesPermissions onboarding consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt (1)
398-405: ⚡ Quick winNarrow exception handling in
openPrivacyPolicy().Line 403 catches
Exception, which can hide unrelated runtime failures. Catch the specific launch failure (ActivityNotFoundException) and keep other exceptions visible.Proposed patch
+import android.content.ActivityNotFoundException import android.content.Context import android.content.Intent @@ private fun openPrivacyPolicy() { - try { - val privacyPolicyUrl = getString(R.string.privacy_policy_url) - val intent = Intent(Intent.ACTION_VIEW, privacyPolicyUrl.toUri()) - startActivity(intent) - } catch (e: Exception) { - Sentry.captureException(e) - } + val privacyPolicyUrl = getString(R.string.privacy_policy_url) + val intent = Intent(Intent.ACTION_VIEW, privacyPolicyUrl.toUri()) + try { + startActivity(intent) + } catch (e: ActivityNotFoundException) { + Sentry.captureException(e) + } }Based on learnings: In Kotlin files, prefer narrow exception handling that catches only the specific exception type reported in crashes (such as IllegalArgumentException) instead of a broad catch-all (e.g., catch (e: Exception)).
🤖 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/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt` around lines 398 - 405, In openPrivacyPolicy(), narrow the broad catch block so you only handle the launch-specific failure: replace catch (e: Exception) with catch (e: ActivityNotFoundException) (import it), call Sentry.captureException(e) there, and let any other exceptions (e.g., IllegalArgumentException from toUri() or other runtime errors) propagate so they aren't accidentally swallowed; locate the code around privacyPolicyUrl, Intent(Intent.ACTION_VIEW, privacyPolicyUrl.toUri()), and startActivity(intent) to make the change.
🤖 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.
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt`:
- Around line 398-405: In openPrivacyPolicy(), narrow the broad catch block so
you only handle the launch-specific failure: replace catch (e: Exception) with
catch (e: ActivityNotFoundException) (import it), call
Sentry.captureException(e) there, and let any other exceptions (e.g.,
IllegalArgumentException from toUri() or other runtime errors) propagate so they
aren't accidentally swallowed; locate the code around privacyPolicyUrl,
Intent(Intent.ACTION_VIEW, privacyPolicyUrl.toUri()), and startActivity(intent)
to make the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 529b68ef-bef0-44ca-bf05-7f4dcb69423e
📒 Files selected for processing (11)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.ktapp/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.ktapp/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/onboarding/PermissionInfoAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsInfoFragment.ktapp/src/main/java/com/itsaky/androidide/models/PermissionInfoItem.ktapp/src/main/res/layout/fragment_permissions_info.xmlapp/src/main/res/layout/layout_permission_info_item.xmlresources/src/main/res/values-in-rID/strings.xmlresources/src/main/res/values/strings.xml
💤 Files with no reviewable changes (10)
- app/src/main/res/layout/fragment_permissions_info.xml
- app/src/main/res/layout/layout_permission_info_item.xml
- app/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.kt
- resources/src/main/res/values/strings.xml
- app/src/main/java/com/itsaky/androidide/models/PermissionInfoItem.kt
- resources/src/main/res/values-in-rID/strings.xml
- app/src/androidTest/kotlin/com/itsaky/androidide/helper/OnboardingPermissionsInfoHelper.kt
- app/src/main/java/com/itsaky/androidide/adapters/onboarding/PermissionInfoAdapter.kt
- app/src/androidTest/kotlin/com/itsaky/androidide/scenarios/NavigateToMainScreenScenario.kt
- app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsInfoFragment.kt
EndToEndTest had three steps hardcoded to the deleted permissions-info slide that would have asserted against missing R.id.intro_text / R.id.permissions_list or thrown looking for a NEXT button on the (now chrome-hidden) Permissions slide. The privacy dialog assertions now run directly against the Permissions slide after advancePastWelcomeScreen(). The orphan PermissionsInfoScreen Kaspresso object is also removed.
jomen-adfa
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Streamlines onboarding from three slides to two by dropping the informational "Next page: permissions" slide. The Privacy & analytics disclosure dialog — previously triggered on slide 2's onSlideSelected — now fires on the Permissions slide via the same listener hook, preserving the once-per-install preference key so existing users aren't re-prompted.
Removes the now-unused fragment, layout, adapter, model, test helper, and the five permissions_info_* strings (en + id).