Add new onboarding dialog to preview input mode toggle#8206
Add new onboarding dialog to preview input mode toggle#8206
Conversation
LukasPaczos
left a comment
There was a problem hiding this comment.
- Rotating the screen while the search/chat suggestions are animating is crashing.
Stacktrace
FATAL EXCEPTION: main
Process: com.duckduckgo.mobile.android.debug, PID: 22327
java.lang.IllegalStateException: Can't access the Fragment View's LifecycleOwner for WelcomePage{3b5835d} (e873eddd-7c95-40db-b890-bcbda61c9d08) when getView() is null i.e., before onCreateView() or after onDestroyView(
at androidx.fragment.app.Fragment.getViewLifecycleOwner(Fragment.java:390)
at com.duckduckgo.common.ui.viewbinding.FragmentViewBindingDelegate.getValue(FragmentViewBindingDelegate.kt:78)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage.getBinding(WelcomePage.kt:97)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage.access$getBinding(WelcomePage.kt:85)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage$configureDaxCta$1$20.invoke$lambda$1$animateButton(WelcomePage.kt:560)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage$configureDaxCta$1$20.invoke$lambda$1$animateButton$lambda$0(WelcomePage.kt:567)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage$configureDaxCta$1$20.$r8$lambda$TEzRAOCg8jk2cXYTIkoNyq0JUtw(Unknown Source:0)
at com.duckduckgo.app.onboarding.ui.page.WelcomePage$configureDaxCta$1$20$$ExternalSyntheticLambda0.run(D8$$SyntheticClass:0)
at android.view.ViewPropertyAnimator$AnimatorEventListener.onAnimationEnd(ViewPropertyAnimator.java:1120)
at android.animation.Animator$AnimatorListener.onAnimationEnd(Animator.java:784)
at android.animation.Animator$AnimatorCaller$$ExternalSyntheticLambda1.call(D8$$SyntheticClass:0)
at android.animation.Animator.callOnList(Animator.java:742)
at android.animation.Animator.notifyListeners(Animator.java:640)
at android.animation.Animator.notifyEndListeners(Animator.java:665)
at android.animation.Animator.completeEndAnimation(Animator.java:708)
at android.animation.ValueAnimator.completeEndAnimation(ValueAnimator.java:1336)
at android.animation.Animator.notifyEndListenersFromEndAnimation(Animator.java:697)
at android.animation.ValueAnimator.endAnimation(ValueAnimator.java:1322)
at android.animation.ValueAnimator.doAnimationFrame(ValueAnimator.java:1592)
at android.animation.AnimationHandler.doAnimationFrame(AnimationHandler.java:404)
at android.animation.AnimationHandler.-$$Nest$mdoAnimationFrame(Unknown Source:0)
at android.animation.AnimationHandler$1.doFrame(AnimationHandler.java:106)
at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1645)
at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1656)
at android.view.Choreographer.doCallbacks(Choreographer.java:1252)
at android.view.Choreographer.doFrame(Choreographer.java:1177)
at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1630)
at android.os.Handler.handleCallback(Handler.java:1095)
at android.os.Handler.dispatchMessageImpl(Handler.java:135)
at android.os.Handler.dispatchMessage(Handler.java:125)
at android.os.Looper.loopOnce(Looper.java:269)
at android.os.Looper.loop(Looper.java:367)
at android.app.ActivityThread.main(ActivityThread.java:9333)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:566)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:929)
- According to designs, the input field should be focused and keyboard should be up when the new onboarding step is presented. This might be problematic in landscape though, as focusing the input text field obscures the whole window right now.
- The touch area for the search/submit button seems to span well over the actual button size:
Screen_recording_20260408_094841.mp4
- Should we limit the input box to be single line only? Having it multi-line in this scenario could be a little awkward:
Screen_recording_20260408_095103.mp4
- In landscape, the bubble with suggestions is clipped and can't be scrolled down enough to reveal the whole shape:
4 and 5 might need more discussion, so we can follow up separately.
| val inputScreenPreviewBinding = binding.daxDialogCta.inputScreenPreview | ||
|
|
||
| listOf(inputScreenPreviewBinding.suggestion1, inputScreenPreviewBinding.suggestion2, inputScreenPreviewBinding.suggestion3) | ||
| .forEachIndexed { index, button -> suggestions[index].setOptionView(button) } |
There was a problem hiding this comment.
nit: this assumes the provided suggestions list has exactly 3 elements, otherwise it'd crash. Not a problem today but a little fragile.
There was a problem hiding this comment.
Technically, it assumes the list of suggestions has at least 3 elements. If we didn't make any assumptions about the number of suggestions, we would have to handle quite a few edge cases (if not here than in other places in the code), and it's not always clear what would be the expected behavior. I'm not sure it is worth the extra effort/complexity.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d51a9c. Configure here.
| val inputScreenPreviewBinding = binding.daxDialogCta.inputScreenPreview | ||
|
|
||
| listOf(inputScreenPreviewBinding.suggestion1, inputScreenPreviewBinding.suggestion2, inputScreenPreviewBinding.suggestion3) | ||
| .forEachIndexed { index, button -> suggestions[index].setOptionView(button) } |
There was a problem hiding this comment.
Unchecked index access assumes exactly three suggestions
Low Severity
setInputScreenPreviewInputMode directly indexes into suggestions[index] for indices 0, 1, 2 without bounds checking. If suggestions has fewer than 3 elements, this throws an IndexOutOfBoundsException. Currently getSearchOptions() and getChatSuggestions() both return exactly 3 items so it works, but the coupling between the list size and the hardcoded 3 buttons is implicit and fragile — any future change to those provider methods would cause a runtime crash.
Reviewed by Cursor Bugbot for commit 4d51a9c. Configure here.
This is just an odd side effect of the button not having a click listener at the moment. It won't be an issue in the final implementation. |
Task/Issue URL: https://app.asana.com/1/137249556945/project/1205648422731273/task/1213970480206403?focus=true
Description
Steps to test this PR
DuckAiOnboardingExperimentManagerImpl::enroll()to return one of the treatment variantsDuckAiOnboardingExperimentManagerImpl::enroll()UI changes
Note
Medium Risk
Changes onboarding flow control and UI by inserting a new, experiment-gated dialog after the input-screen selection step; risks are mainly around navigation/animation regressions and variant gating logic.
Overview
Adds a new
INPUT_SCREEN_PREVIEWonboarding step that previews the Search vs AI Chat input-mode toggle (with animated suggestions and optional keyboard) via a newpre_onboarding_input_mode_demo.xmlinclude.Updates
WelcomePage/WelcomePageViewModelto optionally show this preview after the input-screen selection, gated by a newDuckAiOnboardingExperimentManagervariant (control/duck-ai-default/search-default); addsOnboardingStore.getChatSuggestions()plus new strings for chat prompts.Adjusts onboarding CTA layout spacing to accommodate the new embedded preview, and updates unit tests to cover experiment outcomes and the new dialog path.
Reviewed by Cursor Bugbot for commit 4d51a9c. Bugbot is set up for automated code reviews on this repo. Configure here.