-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add FXIOS-12322 Implement tracking protection screen in new onboarding with crash & telemetry toggle modal #27217
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
Conversation
…eCardView.swift Co-authored-by: lmarceau <[email protected]>
…le generic Onboarding card
…c-&-Multi-Select-cards-into-a-single-generic-Onboarding-card # Conflicts: # BrowserKit/Sources/OnboardingKit/Preview/PreviewModel.swift # BrowserKit/Sources/OnboardingKit/Views/OnboardingBasicCardView.swift # BrowserKit/Sources/OnboardingKit/Views/OnboardingMultipleChoiceCardView.swift
…c-&-Multi-Select-cards-into-a-single-generic-Onboarding-card
…ding-View-into-Main-Target-with-Nimbus # Conflicts: # BrowserKit/Sources/OnboardingKit/Models/OnboardingFlowViewModel.swift # BrowserKit/Sources/OnboardingKit/Preview/PreviewModel.swift # BrowserKit/Sources/OnboardingKit/Views/OnboardingBasicCardView.swift # BrowserKit/Sources/OnboardingKit/Views/OnboardingCardView.swift # BrowserKit/Sources/OnboardingKit/Views/OnboardingMultipleChoiceCardView.swift # BrowserKit/Sources/OnboardingKit/Views/UX.swift
…ding-View-into-Main-Target-with-Nimbus
…ding with Crash & Telemetry Toggle Modal
…nto-Main-Target-with-Nimbus' into rlitianu/FXIOS-12322-#26842-Implement-Tracking-Protection-Screen-in-New-Onboarding-with-Crash-&-Telemetry-Toggle-Modal
This pull request has conflicts when rebasing. Could you fix it @razvanlitianu? 🙏 |
@@ -159,6 +162,42 @@ extension PreviewModel { | |||
imageID: "toolbar", | |||
instructionsPopup: nil | |||
) | |||
|
|||
static let tos = PreviewModel( |
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.
Dummy data
Client.app: Coverage: 35.37
ComponentLibrary: Coverage: 33.59
Generated by 🚫 Danger Swift against 2c6ee67 |
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.
LGTM!! 😎
@Mergifyio backport release/v140 |
✅ Backports have been created
|
…g with crash & telemetry toggle modal (#27217) * FXIOS-12237 #26639 Implement Multiple-Selection Onboarding card component * Swiftlint fix * Swiftlint fix * Pr review * Update BrowserKit/Sources/OnboardingKit/Views/OnboardingMultipleChoiceCardView.swift Co-authored-by: lmarceau <[email protected]> * PR updates * FXIOS-12238 #26640 Encapsulate Basic & Multi-Select cards into a single generic Onboarding card * Swiftlint fix * swiftlint fix * Style cards * Improve page change logic * FXIOS-12325 #26845 Integrate Onboarding View into Main Target with Nimbus * Add ui variants * Swiftlint fix * Fix naming * cleanup * swiftlint * swiftlint * Swiftlint * Add cards * Change experiment * Turn off modern ui * FXIOS-12322 #26842 Implement Tracking Protection Screen in New Onboarding with Crash & Telemetry Toggle Modal * Swiftlint fix * Reset strings * Filter --------- Co-authored-by: lmarceau <[email protected]> (cherry picked from commit 295fcbc) # Conflicts: # firefox-ios/Client/Frontend/Onboarding/Models/OnboardingKitCardInfoModel.swift
…g with crash & telemetry toggle modal (backport #27217) (#27268) Add FXIOS-12322 Implement tracking protection screen in new onboarding with crash & telemetry toggle modal (#27217) * FXIOS-12237 #26639 Implement Multiple-Selection Onboarding card component * Swiftlint fix * Swiftlint fix * Pr review * Update BrowserKit/Sources/OnboardingKit/Views/OnboardingMultipleChoiceCardView.swift Co-authored-by: lmarceau <[email protected]> * PR updates * FXIOS-12238 #26640 Encapsulate Basic & Multi-Select cards into a single generic Onboarding card * Swiftlint fix * swiftlint fix * Style cards * Improve page change logic * FXIOS-12325 #26845 Integrate Onboarding View into Main Target with Nimbus * Add ui variants * Swiftlint fix * Fix naming * cleanup * swiftlint * swiftlint * Swiftlint * Add cards * Change experiment * Turn off modern ui * FXIOS-12322 #26842 Implement Tracking Protection Screen in New Onboarding with Crash & Telemetry Toggle Modal * Swiftlint fix * Reset strings * Filter --------- Co-authored-by: lmarceau <[email protected]> (cherry picked from commit 295fcbc) # Conflicts: # firefox-ios/Client/Frontend/Onboarding/Models/OnboardingKitCardInfoModel.swift Co-authored-by: Litianu Razvan <[email protected]>
public static let ContinueAction = MZLocalizedString( | ||
key: "Onboarding.Modern.Customization.Theme.Continue.Action.v123", | ||
key: "Onboarding.Modern.Customization.Theme.Continue.Action.v140", |
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.
Hey! Just stumbled on this. This string was already exported and only changing the key will effectively remove all translations for that string. Why are we changing the key here?
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.
Hey @lmarceau, I apologize for the oversight. I was supposed to add 140, but I accidentally copied the wrong value. I thought I could modify the string before exporting, but if I missed it, I’ll have to add a new key. Do you happen to know if the export was already completed?
title: "Upgrade your browsing", | ||
body: "Our fastest iOS browser yet\nAutomatic tracking protection\nSync on all your devices", |
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.
Can we double check those strings don't get exported to l10n? We had an issue at one point where some SwiftUI preview string gets exported. I just wanna make sure this isn't the case here
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.
We used Text(verbatim:
in those specific places 👀
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.
In this case, the use of \n is intentional. We configure the onboarding cards from Nimbus, which have a title and a subtitle. However, there’s no option to add an array of strings for the subtitle, so we have to format it as shown.
📜 Tickets
Jira ticket
Github issue
💡 Description
Create a modern new UI for the Terms of Service screen. Create the view and configure it with the other onboarding cards. Integrate it into the main targeted screen and present it using the flag.
🎥 Demos
Demo
📝 Checklist
@Mergifyio backport release/v120
)