Removed the experiment for the new default browser dialog for full release #1072
Conversation
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
aitorvs
force-pushed
the
feature/aitor/new_default_browser_dialog_all_users
branch
from
aaf432a to
d74b9f4
Compare
marcosholgado
requested changes
Jan 25, 2021
Contributor
marcosholgado
left a comment
marcosholgado
left a comment
There was a problem hiding this comment.
Nice job. I left a few comments to rename tests and some methods and classes since we don't have an experiment or variants anymore. Other things to rename that are not in the PR are:
WelcomePageViewModelTesthasXpin the test names which will make no sense when renamingshouldShowExperiment.DefaultRoleBrowserDialogExperimenthas a method calledexperimentShownthat we should also rename to something likedialogShownor similar.VariantManagerhas the unused methodisAtLeastApiVersion
app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/OnboardingPageManagerPageCountTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/OnboardingPageManagerTest.kt
Outdated
Show resolved
Hide resolved
app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/OnboardingPageManagerTest.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/duckduckgo/app/onboarding/di/WelcomePageModule.kt
Outdated
Show resolved
Hide resolved
aitorvs
force-pushed
the
feature/aitor/new_default_browser_dialog_all_users
branch
2 times, most recently
from
46a263b to
712c88b
Compare
Collaborator
Author
|
@marcosholgado thanks! I think I addressed all comments in the latest commit |
* Renamed DefaultRoleBrowserDialogExperiment -> DefaultRoleBrowserDialog * Renamed shouldShowExperiment -> shouldShowDialog * Renamed experimentShown -> dialogShown * Removed isAtLeastApiVersion * Removed references to browser dialog variant in tests
aitorvs
force-pushed
the
feature/aitor/new_default_browser_dialog_all_users
branch
from
712c88b to
56f3cba
Compare
|
Does this PR lead to a new OOBE? |
aitorvs
deleted the
feature/aitor/new_default_browser_dialog_all_users
branch
Contributor
@Cyberdroid1, If by OOBE you mean out of the box experience then the answer is yes. |
|
Yes, I mean the out of box experience. I would be looking forward to it! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Task/Issue URL: https://app.asana.com/0/414730916066338/1199628713920165/f
Tech Design URL:
CC: https://app.asana.com/0/1142021229838617/1199633113249889/f
Description:
After the successfull experiment, see https://app.asana.com/0/0/1199361498013999/f, this PR:
zuandztvariantsDefaultRoleBrowserDialogExperiment#shouldShowExperimentto remove the variant checkOnboardingPageManagerWithTrackerBlockingto remove the variant check and to ensure it shows when the "new Chrome default browser dialog" does notSteps to test this PR:
==Test case 1==
Device API >=29
==Test case 2==
==Test case 3==
==Test case 4==
Internal references:
Software Engineering Expectations
Technical Design Template