ADFA-5097: Clean up telemetry choice screen - #1658
Merged
Merged
Conversation
The consent dialog was a stock MaterialAlertDialog built from setTitle + setMessage + three buttons. Every complaint in the ticket was that widget's behavior once the message overflowed at large font scale: Material draws scroll-indicator dividers around the message pane, the borderless text buttons read as plain text, and the message scrolls while the button bar stays fixed. On the reported device the button bar was pushed off-screen entirely, leaving "Keep offline" half-cut and "Learn more" invisible - the decline option was unreachable on a non-cancelable dialog. Styling could not fix that, so the dialog now takes a custom view holding the body and both choices in one NestedScrollView: - No dividers. Material only draws them around the message pane, and there is no longer a message. - Choices are real filled/outlined MaterialButtons, full width, stacked. - Body and choices scroll together, and the scrollbar is non-fading so scrollability is visible before the user touches anything. - The choice panel sits on colorSurfaceVariant to set it off from the body. Not a surfaceContainer role: this app's themes define colorSurfaceVariant but leave the container roles to the Material3 defaults, and a Material3 dialog is itself colorSurfaceContainerHigh, so such a panel would be invisible. Copy condensed to two paragraphs, dropping the Firebase/GlitchTip names and the privacy-policy paragraph. At font scale 2.0 it now fits without scrolling at all. "Learn more" is removed. It opened an off-device PDF via ACTION_VIEW - exactly what the app tries to avoid - and the content is legal boilerplate already on the website. privacy_policy_url had no other consumer and is deleted too. Also retranslates the zh-rCN and in-rID strings, which were not merely stale: both were missing privacy_disclosure_decline entirely and rendered "accept" as "I understand" / "Saya mengerti", so those users saw a one-button dialog whose button said something the English never said. Verified on an arm64 emulator at font scale 1.0 and 2.0, plus 3.0 to force overflow and confirm the scrollbar appears and both buttons scroll into reach. Light and dark themes checked; accept persists GRANTED and decline persists DECLINED.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 Walkthrough
WalkthroughThe privacy consent dialog now uses a custom layout with updated localized disclosure text. It offers anonymous-data sharing or offline operation. The Learn more action and related test assertions were removed while consent persistence remains. ChangesPrivacy consent flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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.
Actionable comments posted: 1
🤖 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/res/layout/layout_dialog_privacy_consent.xml`:
- Around line 39-51: Add meaningful android:contentDescription values to the
MaterialButtons privacy_accept and privacy_decline using new cd_* string
resources, then register the required long-press tooltips for both controls in
PermissionsFragment where their listeners are bound, following the existing
three-tier idetooltips pattern.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b979853-d93d-40ec-9d34-9b2dce0caf35
📒 Files selected for processing (6)
app/src/androidTest/kotlin/com/itsaky/androidide/helper/HandlePrivacyDisclosureHelper.ktapp/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.ktapp/src/main/res/layout/layout_dialog_privacy_consent.xmlresources/src/main/res/values-in-rID/strings.xmlresources/src/main/res/values-zh-rCN/strings.xmlresources/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- app/src/androidTest/kotlin/com/itsaky/androidide/helper/HandlePrivacyDisclosureHelper.kt
Paragraph 2 names the two choices; setting them in italics makes them stand out and hints at the buttons below. Inline <i> markup in the string resource. The layout binds via android:text, so TextView picks up the style spans directly - getString() would have stripped them, but nothing here calls it. Not applied to zh-rCN: synthetic obliquing of Han characters renders poorly, and that translation already sets the two choice names off with the conventional CJK quotation marks.
Daniel-ADFA
approved these changes
Aug 11, 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.
Fixes ADFA-5097.
Root cause
All five complaints trace to one thing: the consent screen was a stock
MaterialAlertDialogbuilt fromsetTitle+setMessage+ three buttons. Once the message overflows at large font scale, Material's own dialog produces exactly the reported symptoms.scrollIndicatorUp/Down, shown only when the message pane overflowsACTION_VIEWto a PDF on appdevforall.orgWorth calling out: in the reported screenshot the button bar is pushed off-screen, so "Keep offline" is half-cut and "Learn more" is gone. On a
setCancelable(false)dialog the user could not see or reach the decline option at all.Items 1, 3 and 7 are properties of the widget, not of its styling, so the dialog now supplies its whole body as a custom view with the choices inside the scroll container.
Changes
layout_dialog_privacy_consent.xml- oneNestedScrollViewholding the body text and both choices. No dividers (Material only draws them around a message pane, and there is no longer a message).fadeScrollbars="false"so the scrollbar is visible before the user touches anything. Choices are full-width filled/outlinedMaterialButtons.?attr/colorSurfaceVariant, not a surfaceContainer role. This app's themes definecolorSurfaceVariantper-theme but leave the container roles to the Material3 defaults - and a Material3 dialog iscolorSurfaceContainerHigh, so the panel was invisible until this was changed.PermissionsFragment.showPrivacyDialog()rewritten;openPrivacyPolicy()deleted. Now usesDialogUtils.newMaterialDialogBuilder()- this dialog was the only one in the app built with a bareMaterialAlertDialogBuilder, so it was silently missing the app's corner radius and enter/exit animators. Inflated frombuilder.contextso the dialog-scoped theme attributes resolve.privacy_policy_urlhad no other consumer and is deleted.privacy_disclosure_declineentirely and rendered accept as "I understand" / "Saya mengerti", so those users saw a one-button dialog whose button said something the English never said. Round-tripped back to English viatranslate-strings-xml.py- no semantic drift,\n\nescapes intact. (Only the Gemini leg ran; the Google Cloud Translate leg needsgcloud auth application-default login.)isClickable, so paragraph 2 repeating the button labels does not confuse it.Verification
Built and exercised on an arm64 emulator, fresh install each run:
GRANTED, decline persistsDECLINED(verified inide.stats.xml); dialog does not reappear.:app:assembleV8Debug,:app:compileV8DebugAndroidTestKotlin, andspotlessCheckall pass.Screenshots at each scale are in the ticket.