-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add FXIOS-14451 #31297 Configure brand resfresh onboarding #31505
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
Add FXIOS-14451 #31297 Configure brand resfresh onboarding #31505
Conversation
|
This pull request has conflicts when rebasing. Could you fix it @razvanlitianu? 🙏 |
1 similar comment
|
This pull request has conflicts when rebasing. Could you fix it @razvanlitianu? 🙏 |
c66cf0e to
ac113ed
Compare
073c10f to
e01d3d1
Compare
…rand-refresh-v148-config
- Change Terms of Service button action from syncSignIn to endOnboarding - Add documentation for onboarding variant priority logic - Set should-use-brand-refresh-configuration to false by default
FilippoZazzeroni
left a comment
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.
Hi @razvanlitianu LGTM only a question, do we need to add all the images back for the brand refresh variant, i see expect for the sync card that the images are the same as japan onboarding, couldn't we add just the only one different ?
| case .themeLightBrandRefresh: return ImageIdentifiers.Onboarding.MultipleChoiceButtonImages.themeLightBrandRefresh | ||
| case .toolbarTopBrandRefresh: return ImageIdentifiers.Onboarding.MultipleChoiceButtonImages.toolbarTopBrandRefresh | ||
| case .toolbarBottomBrandRefresh: | ||
| return ImageIdentifiers.Onboarding.MultipleChoiceButtonImages.toolbarBottomBrandRefresh |
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.
nit: can we put it on the same line like the other cases
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 @FilippoZazzeroni it fails the Swiftlint check so that's why it is a bit off.
So technically no. I was doing this for consistency but I see how it might actually increase the app size. I will take a second look and see what can be reused. |
Thanks very much @razvanlitianu 😃 |
|
@mergify rebase |
❌ Base branch update has failedDetailsGit reported the following error: |
…rand-refresh-v148-config
… and Brand Refresh variants - Reuse Japan theme and toolbar images for Brand Refresh variant (images are identical) - Keep trackers-brand-refresh and sync-with-icons-brand-refresh separate (variant-specific) - Remove duplicate Brand Refresh asset files (~1.5MB reduction) - Update FML configuration to reference shared images - Remove unused ImageIdentifiers constants and enum case handlers - Update descriptions to clarify which images are shared vs variant-specific
🥇 Perfect PR sizeSmaller PRs are easier to review. Thanks for making life easy for reviewers! ✨ ✅ Per-file coverageAll changed files meet the threshold of 35.0%. Client.app: Coverage: 37.28
Generated by 🚫 Danger Swift against b643be4 |
FilippoZazzeroni
left a comment
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 @razvanlitianu 😃
| @@ -1 +1 @@ | |||
| APP_VERSION = 147.1 | |||
| APP_VERSION = 147.2 | |||
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.
nit: we can remove this file
|
🚀 PR merged to |
📜 Tickets
Jira ticket
Github issue
💡 Description
Changes
Adds brandRefresh onboarding variant with feature flag support
Adds 7 new PDF image assets for brand refresh (sync, trackers, theme variants, toolbar variants)
Adds Nimbus configuration for brand refresh onboarding cards using v148 localized strings
🎥 Demos
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-01-12.at.12.52.05.mov
Demo
📝 Checklist