Onboarding optimizations #1089

marcosholgado · 2021-02-05T10:44:45Z

Finish animation if dialog is not dismissible when the user clicks outisde.

Task/Issue URL: https://app.asana.com/0/715106103902962/1199405563917479
Tech Design URL:
CC:

Description:
This PR adds a few onboarding optimizations:

Makes the Dax Dialogs non-dismissible and finishes the typing animation when the user clicks outside of them. These changes do not apply to the fire button dialog.
Changes the behaviour of the welcome page so if a user clicks in the background the animation will begin immediately, if the animation was already running it finishes the animation.
Changes the icon used in the onboarding dialogs (also the same we have in the location dialog).

Steps to test this PR:

New dax icon

In the Welcome screen and Dax dialogs the new dax icon should show, the new one has an orange outer circle.

Welcome Page

Install the app
Pressing anywhere (apart from the welcome text) should begin the fade in animation if it hasn't started yet.
Once the animation has started, pressing anywhere in the background should finish the typing animation.

Home screen

Once you press Let's do it in the welcome screen you will be taken to the home screen.
Pressing anywhere around the Dax CTA (blank space) should finish the animation.

Click outside doesn't dismiss first time

Go to wikipedia.org and before the dialog animation finishes, click outside of the dialog.
The dialog should not be dismissed and the tying animation should finish.
Tapping outside again will not dismiss the dialog.

Waiting for the animation to finish

Go to SERP.
Wait for the animation to finish
Tapping outside should not dismiss the dialog.

Finish animation by clicking text

Go to google.com and before the dialog animation finishes, click inside the dialog.
The dialog should not be dismissed and the typing animation should finish.
Tapping outside of the dialog should not dismiss the dialog.

New icon in location dialog

This is a side effect of changing the dax icon.
Go to maps.google.com.
Click in your location in the bottom right corner of the map.
The location dialog show should up.
Notice how the icon has an orange outer circle.

Internal references:

Software Engineering Expectations
Technical Design Template

Finish animation if dialog is not dismissible when the user clicks outisde.

cmonfortep · 2021-02-09T10:43:02Z

Hey @marcosholgado, please ping me here once the product review is done. thanks!

app/src/main/java/com/duckduckgo/app/cta/ui/Cta.kt

app/src/main/java/com/duckduckgo/app/global/view/DaxDialog.kt

marcosholgado · 2021-02-10T09:50:32Z

Hey @marcosholgado, please ping me here once the product review is done. thanks!

@cmonfortep, product review approved! This is now ready.

cmonfortep · 2021-02-11T08:46:51Z

@marcosholgado while testing this I've noticed there's a ripple effect when clicking outside the dialog. It feels weird to me to have that effect there. Is this something intentional?

cmonfortep

Code looks great, just some minor comments. I will wait for that final check on the ripple effect.

cmonfortep · 2021-02-11T08:57:56Z

app/src/main/java/com/duckduckgo/app/global/view/TypeAnimationTextView.kt

    private var typingAnimationJob: Job? = null
    private var delayAfterAnimationInMs: Long = 300
    var typingDelayInMs: Long = 20
+    lateinit var textInDialog: Spanned


No strong opinion here, but I think is safer to use a nullable type in here instead of a lateinit. Since this will be initialized by startTypingAnimation but there are other public methods that could be called by mistake before that, I will put a safety net and go for nullable instead.

I think it makes sense, yes. You could be calling finish animation when textInDialog is not initialized which it will crash the app. Changed!

app/src/main/java/com/duckduckgo/app/global/view/DaxDialog.kt

cmonfortep · 2021-02-11T09:09:57Z

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

+            newTabLayout.setOnClickListener { daxCtaContainer.dialogTextCta.finishAnimation() }
+        }
+
+        private fun resetNewTabLayoutClickListener() {


what about removeNewTabLayoutClickListener? reset sounds more like you are going to set a new one instead of just removing the listener.

Makes sense, done!

marcosholgado · 2021-02-11T10:49:14Z

Thanks @cmonfortep, this should be ready to re-review. I was only able to reproduce the ripple in effect on emulators but in any cases I made that foreground invisible to avoid it.

cmonfortep · 2021-02-12T10:25:38Z

app/src/main/java/com/duckduckgo/app/global/view/DaxDialog.kt

            secondaryButtonText: String? = "",
            toolbarDimmed: Boolean = true,
-            dismissible: Boolean = true,
+            dismissible: Boolean = false,


Thanks, this is perfect, but I would also change line 53 in this same file. Just to keep this aligned with default property

👍 changed!

cmonfortep

LGTM, just the comment related to also changing the dismissable value in DaxDialog.ktL53

Good work!

marcosholgado · 2021-02-12T10:41:43Z

LGTM, just the comment related to also changing the dismissable value in DaxDialog.ktL53

Good work!

Good catch! I've just changed it and will merge once it passes CI. Thanks!

marcosholgado added 5 commits February 5, 2021 10:38

Do not dismiss dax dialogs when clicking outside.

6525e42

Finish animation if dialog is not dismissible when the user clicks outisde.

Second tap dismisses the dialog

4afff97

If animation has finished, tapping outside dismisses the dialog.

341304b

Finish animation for Dax CTAs

96a9477

Undo unnecessary change

44ed20d

Welcome page new behaviour and new icon

08b15ab

marcosholgado changed the title ~~Do not dismiss dax dialogs when clicking outside.~~ Onboarding optimizations Feb 10, 2021

marcosholgado added 2 commits February 10, 2021 09:37

Change variable name

3977706

spotless

98069b2

marcosholgado commented Feb 10, 2021

View reviewed changes

app/src/main/java/com/duckduckgo/app/cta/ui/Cta.kt Show resolved Hide resolved

marcosholgado commented Feb 10, 2021

View reviewed changes

app/src/main/java/com/duckduckgo/app/global/view/DaxDialog.kt Show resolved Hide resolved

marcosholgado requested a review from cmonfortep February 10, 2021 09:49

marcosholgado assigned cmonfortep Feb 10, 2021

cmonfortep reviewed Feb 11, 2021

View reviewed changes

Amend code as per PR comments

2adf646

marcosholgado requested a review from cmonfortep February 11, 2021 10:48

cmonfortep reviewed Feb 12, 2021

View reviewed changes

cmonfortep approved these changes Feb 12, 2021

View reviewed changes

Change default value for dismissible

7ddf81a

marcosholgado merged commit a3558c0 into develop Feb 12, 2021

marcosholgado deleted the feature/marcos/make_dax_dialog_non_dismissable branch February 12, 2021 11:39

Onboarding optimizations #1089

Onboarding optimizations #1089

Uh oh!

Conversation

marcosholgado commented Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Internal references:

Uh oh!

cmonfortep commented Feb 9, 2021

Uh oh!

Uh oh!

Uh oh!

marcosholgado commented Feb 10, 2021

Uh oh!

cmonfortep commented Feb 11, 2021

Uh oh!

cmonfortep left a comment

Choose a reason for hiding this comment

Uh oh!

cmonfortep Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

marcosholgado Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cmonfortep Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

marcosholgado Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

marcosholgado commented Feb 11, 2021

Uh oh!

cmonfortep Feb 12, 2021

Choose a reason for hiding this comment

Uh oh!

marcosholgado Feb 12, 2021

Choose a reason for hiding this comment

Uh oh!

cmonfortep left a comment

Choose a reason for hiding this comment

Uh oh!

marcosholgado commented Feb 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marcosholgado commented Feb 5, 2021 •

edited

Loading