-
Notifications
You must be signed in to change notification settings - Fork 426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Contextual Onboarding Linear #3089
Contextual Onboarding Linear #3089
Conversation
🚫 The Asana task linked in the PR description is not added to iOS App Board project.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linear Flow works as expected!
If you visit a site and then do a search, the search contextual appears and after pressing got it still shows the site suggestion.
I left a comment on this: showNextScreen in OnboardingFirstSearchDoneDialog should be set accordingly to whether a site has been visited or not
let message = NSAttributedString(string: UserText.DaxOnboardingExperiment.ContextualOnboarding.onboardingFirstSearchDoneMessage) | ||
let cta = UserText.DaxOnboardingExperiment.ContextualOnboarding.onboardingGotItButton | ||
|
||
@State private var showNextScreen: Bool = false | ||
@State private var shouldFollowUp: Bool = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn’t this come from outside? (based on wether or not a website has already been visited?
4a0a337
to
df50cb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great now! thanks!
I left 2 very minor comments
DuckDuckGo/DaxDialogs.swift
Outdated
guard let host = privacyInfo.domain else { return nil } | ||
|
||
if privacyInfo.url.isDuckDuckGoSearch { | ||
// If user already performed a search, show after search dialog but don't follow up by suggesting to visit a site. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess here should be “If user already visited a site..."
try super.setUpWithError() | ||
delegateMock = MockTabDelegate() | ||
onboardingPresenterMock = ContextualOnboardingPresenterMock() | ||
sut = .mock(contextualOnboardingPresenter: onboardingPresenterMock) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not call this tab mock cause it’s a real tab I’d call it fixture? A standard initialisation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to "fake"
As discussed I merged #3093 in here feel free to merge this when you have into the next branch etc 🥲 |
b7e6942
to
5709cf7
Compare
…s pressed and general clean up
…and updated OnboardingNavigationDelegate tests
Task/Issue URL: https://app.asana.com/0/0/1207812824983911/f https://app.asana.com/0/0/1207812824983904/f Description: Makes the Dialogs scrollable and refactors the ContextualDialogs to support expansion animation
e6e9b78
to
5afa6b9
Compare
Thanks, I’ve fixed them! |
8e3e8ad
into
AleSab/implement-contextual-onboarding
Co-authored-by: Sabrina Tardio <[email protected]>
Co-authored-by: Sabrina Tardio <[email protected]>
Co-authored-by: Sabrina Tardio <[email protected]>
Co-authored-by: Sabrina Tardio <[email protected]>
Task/Issue URL: https://app.asana.com/0/1206329551987282/1207812824986102/f
Description:
Show the Contextual linear flow.
Note
I will add tests in a subsequent PR. I wanted to unblock you so you can use this branch.
Steps to test this PR:
newInstallCompletion(self)
inDefaultVariantManager
at line 135mb
Definition of Done (Internal Only):
Copy Testing:
’
rather than'
Orientation Testing:
Device Testing:
OS Testing:
Theme Testing:
Internal references:
Software Engineering Expectations
Technical Design Template