Onboarding Highlights Ship Review #3380

alessandroboron · 2024-09-23T08:42:31Z

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208291253556033/f

Description:

This PR includes changes and fixes that have been addressed during the Ship Review.

Commits are mostly self contained and address each Ship Review comment, so you can review each commit separately if you prefer.
The PR also includes the copy translations.

Steps to test this PR:
Test Scenario 1, 2, and 3 of Ship Review Testing section

Definition of Done (Internal Only):

Does this PR satisfy our Definition of Done?

Copy Testing:

Use of correct apostrophes in new copy, ie ’ rather than '

Orientation Testing:

Portrait
Landscape

Device Testing:

OS Testing:

iOS 15
iOS 16
iOS 17

Theme Testing:

Light theme
Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

github-actions · 2024-09-23T08:44:13Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

	Messages
📖	You seem to be updating localized strings. Make sure that you request translations and include translated strings before you ship your change. See Localization Guidelines for more information.

Generated by 🚫 dangerJS against 5435710

brindy

Added a note to the asana task with the scenarios in them. Couple of comments here too.

brindy · 2024-09-23T14:20:50Z

Core/Debouncer.swift

+            block()
+        })
+
+        RunLoop.main.add(timer, forMode: .common)


Nothing about this class suggests this is for the main loop. Can that be more explicit? Given you've put it in a common location I'd be tempted to also pass in the RunLoop to use, maybe default it to .current which is what I would expect as a dev tbh.

fair point, I will update this and give the dev ability to choose which RunLoop to use.

updated, I’ve also added the RunLoop.Mode as parameter

brindy · 2024-09-23T14:25:13Z

Core/Debouncer.swift

+import Foundation
+
+/// A class that provides a debouncing mechanism.
+public final class Debouncer {


Seems old school to implement this with timers when Combine has one built in but:

1/ Then you'd need to pass in the debounce time in the constructor
2/ It's probably just as many lines of code as this

No change required.

I’m fine to change to use Combine under the hood if we want, but yeah probably the lines of code will be very similar.

brindy · 2024-09-23T14:26:44Z

DuckDuckGo/OnboardingExperiment/ProgressBarView.swift

@@ -144,7 +144,7 @@ struct ProgressBarGradient: View {
                    let nextStep = stepInfo.currentStep < stepInfo.totalSteps ? stepInfo.currentStep + 1 : 1
                    stepInfo = OnboardingProgressIndicator.StepInfo(currentStep: nextStep, totalSteps: stepInfo.totalSteps)
                }, label: {
-                    Text("Update Progress")
+                    Text(verbatim: "Update Progress")


Why did this change? verbatim means everyone will see "Update Progress" but shouldn't it be localised?

Ah this is just a SwiftUI preview I built to test the behaviour of the progress bar. When I tap that button I simulate the progress bar increasing its progress. At translation time I noticed it was translating that string.

alessandroboron · 2024-09-24T13:36:53Z

Added a note to the asana task with the scenarios in them. Couple of comments here too.

I addressed the PR feedback and replied to your Asana comments. I also fixed other bugs I found during testing. All explained in Asana task

brindy

Seems to work fine now, except for the bay thing. Am approving but maybe we'll change eBay?

brindy

~~LGTM - tested each variant starts the expected flow~~

brindy

~~LGTM - tested each variant starts the expected flow~~

…ckers

…uent dialog should be presented, reloading the page would show again the dialog

…rowsing data and navigate to the same website before tapping on end of journey CTA

…sing data twice before tapping on the end of journey CTA

… website when opening a new tab and visit again the website

…ked trackers dialog on anonymous searches page

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208084960727003/f **Description**: Setup 3 branches experiment for Onboarding according to [Asana comment](https://app.asana.com/0/0/1208314441219794/1208151149115582/f).

# By Daniel Bernal (12) and others # Via Daniel Bernal (5) and others * main: (46 commits) Release 7.139.0-4 (#3411) Onboarding highlights experiment updates (#3406) fix suggestions performance (#3405) For third party requests differentiate if they are affiliated with first party (#3386) Bump BSK to pull in C-S-S 6.19.0 (#3396) Release 7.139.0-3 (#3399) Onboarding Highlights Ship Review (#3380) add assertions for tabs in suggestions (#3394) Release 7.139.0-2 (#3398) Bump BSK to Include C.S.S 6.17 (#3397) Bump BSK which includes C.S.S 6.17 (#3395) Rever BSK branch Revert "Bump C.S.S" Bump C.S.S translations for Switch to Tab (#3391) Remove Favorites section header from NTP (#3381) Release 7.139.0-1 (#3389) Add origin to /apps URL (#3378) chery pick returning user fix Release 7.138.1-0 (#3388) ... # Conflicts: # DuckDuckGo/AppDependencyProvider.swift

alessandroboron requested a review from brindy September 23, 2024 08:48

brindy requested changes Sep 23, 2024

View reviewed changes

alessandroboron force-pushed the alessandro/onboarding-highlights-ship-review branch from 3020e0d to 4445e45 Compare September 24, 2024 06:46

alessandroboron requested a review from brindy September 24, 2024 13:37

brindy approved these changes Sep 24, 2024

View reviewed changes

brindy approved these changes Sep 26, 2024

View reviewed changes

alessandroboron changed the base branch from main to release/7.139.0 September 26, 2024 10:08

alessandroboron and others added 20 commits September 26, 2024 20:10

Add translations

cd50785

Remove progress bar text that should not be translated

706b0f5

SH1/ Fix Onboarding Address bar position button colors

5becc09

SH2/ Fix search suggestions for US country

209dd06

Mitgate issue where no tracker dialog is shown instead of blocked tra…

0d20514

…ckers

SH5/ Show baby ducklings images

41ffb26

SH6/ Debounce redirects

b71fde3

PH1/ Lowercase "default" string

fea1811

SH3/ Make DuckDuckGo Search bold

9074a20

Remove fixed offset for AppIcon picker

907a1f3

Add translations

c6d2dcc

Fix spacing in App Icons

220bea6

Fix swiftlint issues

5445995

Fix an issue where when the user dismisses a dialog because no subseq…

add3526

…uent dialog should be presented, reloading the page would show again the dialog

Fix an issue that caused fire dialog to show it again when clearing b…

94582cb

…rowsing data and navigate to the same website before tapping on end of journey CTA

Fix an issue that shows the initial NTP dialog if the user clear brow…

9034e9d

…sing data twice before tapping on the end of journey CTA

Fix an issue that cause showing again the Dax dialog for last visited…

10daa7a

… website when opening a new tab and visit again the website

Address PR comments

ebf9470

Dismiss DaxDialogs when using swipe to navigate to avoid showing bloc…

65ae469

…ked trackers dialog on anonymous searches page

Onboarding Highlights - Experiment Setup (#3390)

19e2495

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208084960727003/f **Description**: Setup 3 branches experiment for Onboarding according to [Asana comment](https://app.asana.com/0/0/1208314441219794/1208151149115582/f).

alessandroboron force-pushed the alessandro/onboarding-highlights-ship-review branch from 9bacb29 to 19e2495 Compare September 26, 2024 10:10

Fix Package.resolved

5435710

alessandroboron merged commit 3184192 into release/7.139.0 Sep 26, 2024
13 checks passed

alessandroboron deleted the alessandro/onboarding-highlights-ship-review branch September 26, 2024 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onboarding Highlights Ship Review #3380

Onboarding Highlights Ship Review #3380

alessandroboron commented Sep 23, 2024 •

edited by brindy

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading

brindy left a comment

brindy Sep 23, 2024

alessandroboron Sep 24, 2024

alessandroboron Sep 24, 2024 •

edited

Loading

brindy Sep 23, 2024

alessandroboron Sep 24, 2024

brindy Sep 23, 2024

alessandroboron Sep 24, 2024 •

edited

Loading

alessandroboron commented Sep 24, 2024

brindy left a comment

brindy left a comment •

edited

Loading

brindy left a comment •

edited

Loading

Onboarding Highlights Ship Review #3380

Onboarding Highlights Ship Review #3380

Conversation

alessandroboron commented Sep 23, 2024 • edited by brindy Loading

Internal references:

github-actions bot commented Sep 23, 2024 • edited Loading

brindy left a comment

Choose a reason for hiding this comment

brindy Sep 23, 2024

Choose a reason for hiding this comment

alessandroboron Sep 24, 2024

Choose a reason for hiding this comment

alessandroboron Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

brindy Sep 23, 2024

Choose a reason for hiding this comment

alessandroboron Sep 24, 2024

Choose a reason for hiding this comment

brindy Sep 23, 2024

Choose a reason for hiding this comment

alessandroboron Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

alessandroboron commented Sep 24, 2024

brindy left a comment

Choose a reason for hiding this comment

brindy left a comment • edited Loading

Choose a reason for hiding this comment

brindy left a comment • edited Loading

Choose a reason for hiding this comment

alessandroboron commented Sep 23, 2024 •

edited by brindy

Loading

github-actions bot commented Sep 23, 2024 •

edited

Loading

alessandroboron Sep 24, 2024 •

edited

Loading

alessandroboron Sep 24, 2024 •

edited

Loading

brindy left a comment •

edited

Loading

brindy left a comment •

edited

Loading