Add FXIOS-12222 [Toolbar refactor] CFR to onboard existing users #26650
Conversation
…-onboard-new-users # Conflicts: # firefox-ios/nimbus-features/toolbarRefactorFeature.yaml
|
@Mergifyio backport release/v139 |
|
@Mergifyio backport release/v138 |
🟠 Waiting for conditions to match
|
🟠 Waiting for conditions to match
|
Client.app: Coverage: 35.08
Generated by 🚫 Danger Swift against 681db9d |
| @@ -271,6 +276,10 @@ class BrowserViewController: UIViewController, | |||
| return featureFlags.isFeatureEnabled(.toolbarNavigationHint, checking: .buildOnly) | |||
| } | |||
|
|
|||
| var isToolbarUpdateHintEnabled: Bool { | |||
| return featureFlags.isFeatureEnabled(.toolbarUpdateHint, checking: .buildOnly) | |||
There was a problem hiding this comment.
Can you add this flag in the spreadsheet?
There was a problem hiding this comment.
Are we adding subflags to the spreadsheet as well? This one is underneath toolbar refactor.
| guard let selectedTab = tabManager.selectedTab, | ||
| selectedTab.isFxHomeTab || selectedTab.isCustomHomeTab | ||
| else { return } |
There was a problem hiding this comment.
With previous toolbar CFR (that was removed recently), we had a check to make sure this wouldn't interfere with the intro screen (on iPad the intro screen is shown on the of the homepage).
The check was:
private func presentContextualHint() {
if IntroScreenManager(prefs: profile.prefs).shouldShowIntroScreen { return }
Just wondering if we should have a similar case here?
There was a problem hiding this comment.
Good catch. I'll look into that. Thanks @lmarceau 🙏
| @@ -19,6 +19,11 @@ features: | |||
| If true, enables the navigation contextual hint. | |||
| type: Boolean | |||
| default: false | |||
| toolbar_update_hint: | |||
There was a problem hiding this comment.
I'm glad you made a demo of all those options in that yaml 😆
| @@ -45,6 +45,8 @@ struct ContextualHintEligibilityUtility: ContextualHintEligibilityUtilityProtoco | |||
| hintTypeShouldBePresented = true | |||
| case .navigation: | |||
| hintTypeShouldBePresented = true | |||
| case .toolbarUpdate: | |||
There was a problem hiding this comment.
Similar question, we had a check in the past to make sure the jump back in sync tab CFR wouldn't show at the same time of the toolbar CFR (removed in this same PR). It seems we should add this back?
/// Determine if the CFR for SyncedTab in JumpBackIn is presentable.
///
/// The context hint is presentable when certain conditions are met:
/// - A synced tab appears in Jump Back In
/// - The Toolbar CFR has already been presented or the CFR toolbar flag is disabled
/// - This CFR hasn't already been presented
/// - The Home Tab Banner isn't being displayed (not specified by Product,
/// but the CFR might show when the anchor point isn't on screen)
private var canPresentJumpBackInSyncedTab: Bool {
return shouldCheckToolbarHasShown
}
📜 Tickets
Jira ticket
Github issue
💡 Description
Adds a new CFR to promote the toolbar update.
🎥 Demos
Demo
📝 Checklist
@Mergifyio backport release/v120)