Updated settings

[tomasstrba]

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

Description:
New Settings layout with Privacy Protections, Main Settings, Next Steps and others. It also includes the implementation of pixel experiment for monitoring the usage of old and new Settings to make sure we don't harm the experience.

ℹ️ The experiment will be activated in a follow-up PR after running experiments on iOS are over.

Steps to test this PR:

Test the old Settings

1. Manually set the cohort in PixelExperiment.swift file

var cohort: PixelExperiment? {
    return .control

    guard isInstalled else { return nil }
...

2. Make sure every setting work as expected and is persisted properly

Test the new Settings

1. Manually set the cohort in PixelExperiment.swift file

var cohort: PixelExperiment? {
    return .newSettings

    guard isInstalled else { return nil }
...

2. Make sure every setting work as expected and is persisted properly

Verify pixels

1. Open list of pixels
2. Make sure both old Settings and new Settings emit the relevant pixels

Copy Testing:

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

Orientation Testing:

• Portrait
• Landscape

Device Testing:

• iPhone SE (1st Gen)
• iPhone 8
• iPhone X
• iPhone 14 Pro
• iPad

OS Testing:

• iOS 14
• iOS 15
• iOS 16

Theme Testing:

• Light theme
• Dark theme

---

Internal references:

Software Engineering Expectations
Technical Design Template

[github-actions]

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

Generated by 🚫 dangerJS against 0e305a1

[brindy]

Mostly looking good just a couple of comments to discuss before I approve.

Also:

1/ "About", "Share Feedback", "DuckDuckGo on other platforms" items don't fire pixels - is that intended? (Looks like it is)

2/ Email protection has a pixel in the new settings, but not the old settings?

[brindy]

Was the idea to call the fire function passed in? In the original macOS code (which I see no longer has this) the idea was to be able to mock calls to pixels so that we could test it.

Talking of which - I don't see any unit tests? I think we should because now it's in the code base there's a good chance this will end up being the defacto standard approach moving forward.

[tomasstrba]

Since the experiment uses existing pixels and it adds many pixels on top, the current approach made more sense to me.

• Mocking would create approximately 2x30 methods which would be hard to maintain.
• Adding existing pixels to the experiment or removal of a pixel from the experiment would require to swap the standard Pixel.fire call with the specific method.

Do you think wrapper calls are that important? I don't have a strong opinion and can generate them (uncle GPT can help with that)

[brindy]

I agree with your approach to firing pixels elsewhere, but regarding this class specifically there's a function passed into this class which you can use rather than calling it directly. If it's not needed then please remove it.

But I assume point of that function was that it makes it "easy" to this logic from a unit test as you can do something like:

func testPixelFiredOnEnrolment() {

   // create expectation

   let logic = PixelExperimentLogic() { pixel in
      // assert the expected pixel is fired

     // fulfil expectation 
   }

   logic.install()
   XCTAssertNotNil(logic.cohort)

   // wait for expectations 

}

And I don't see those kind of tests here?

[tomasstrba]

I see, thanks for clarification! 👍 I missed this part. I have modified it slightly and added couple of tests to ensure pixel is fired when a cohort is allocated. Also, there is a verification the experiment logic returns appropriate pixel parameters.

[tomasstrba]

Thank you for the review, @brindy! We're close to the finish line with the design and copy approvals. As soon as the new copy is translated, I'll reach out to you for the final review.

1/ Correct, no pixels are planned for those sections
2/ Are you referring to settingsEmailProtectionOpen? My understanding is that the old Settings don't have that view. The email button in the old Settings is basically 'Manage Account' button in the new Settings and they both don't have a pixel.

[tomasstrba]

@brindy, translations are in place for new Settings and I also corrected the main view which had a rendering problem because of a different length of labels.

Also, this PR contains translations for Privacy Pro, which is expected and approved by @miasma13.

[brindy]

These don't fire pixels on the old version (which might be correct):

• unprotected
• fireproof sites
• application lock
• keyboard
• long press previews
• open links in associated apps
• email
• mac
• windows
• about
• share feedback

I think we maybe had a miscommunication about the firePixel in the PixelExperiment class. Let me know if the latest comment doesn't make sense.

And also one comment about using a quick link instead of raw URL.

Otherwise all looks good - will approve once we iron those out.

[brindy]

I agree with your approach to firing pixels elsewhere, but regarding this class specifically there's a function passed into this class which you can use rather than calling it directly. If it's not needed then please remove it.

But I assume point of that function was that it makes it "easy" to this logic from a unit test as you can do something like:

func testPixelFiredOnEnrolment() {

   // create expectation

   let logic = PixelExperimentLogic() { pixel in
      // assert the expected pixel is fired

     // fulfil expectation 
   }

   logic.install()
   XCTAssertNotNil(logic.cohort)

   // wait for expectations 

}

And I don't see those kind of tests here?

[brindy]

This opens the default browser, should it just open in a tab instead via quickLink?

[tomasstrba]

Good catch! 👍 I was not aware of this. Corrected ✅

[tomasstrba]

@brindy, thanks for clarification, everything is clear now 👍

1) Pixels
Most pixels you mentioned are not important for the experiment, as stated in list of pixels. The email is slightly confusing, because the flow is different in the old Settings. In the end, I decided to include it there too so we have data for comparison in the experiment

2) Unit Tests
You were right, this logic could see some testing, since it's important for the experiment. Added the one you suggested and couple of more to ensure parameters are correct.

3) Quick Links
Good catch! That would break a lot of things! Fixed ✅

The PR is ready for another review. Thanks for patience with this

[brindy]

LGTM!