Skip to content

Conversation

@lucaschifino
Copy link
Collaborator

@lucaschifino lucaschifino commented Nov 13, 2025

MOB-3897

Context

See ticket.

Approach

Implement new Welcome view using SwiftUI and handling the multiple animations.

Other

  • Cleaning up old onboarding Code

  • Tackled MOB-3903 together

  • Leftover TODOs to be handled afterwards separately

    • Reduce the video size (current 1min and 50MB which is unnecessary)
    • Review the transition to NTP as color change is not yet very smooth

Before merging

Checklist

  • I performed some relevant testing on a real device and/or simulator for both iPhone and iPad
  • I wrote Unit Tests that confirm the expected behaviour Removed spy ones as they were failing, will double check
  • I updated only the english localization source files if needed
  • I added the // Ecosia: helper comments where needed
  • I made sure that any change to the Analytics events included in PR won't alter current analytics (e.g. new users, upgrading users)

@github-actions
Copy link

github-actions bot commented Nov 13, 2025

PR Reviewer Guide 🔍

(Review updated until commit a2df0be)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Leak

The Combine sink and NotificationCenter observer rely on Coordinator deinit for cleanup; verify lifecycle guarantees of the UIViewRepresentable so observers are removed when the view is dismissed to avoid retained player/observers.

// Monitor player status
context.coordinator.statusObserver = playerItem.publisher(for: \.status)
    .receive(on: DispatchQueue.main)
    .sink { status in
        switch status {
        case .readyToPlay:
            player.play()
            context.coordinator.onReady?()
        case .failed:
            // Call onReady even on failure to prevent animations from never starting
            context.coordinator.onReady?()
        case .unknown:
            break
        @unknown default:
            break
        }
    }

// Monitor buffer status
let bufferObserver = playerItem.publisher(for: \.isPlaybackBufferFull)
    .receive(on: DispatchQueue.main)
    .sink { _ in
        // Buffer monitoring
    }

context.coordinator.bufferObserver = bufferObserver

// Loop video when it ends
context.coordinator.loopObserver = NotificationCenter.default.addObserver(
    forName: .AVPlayerItemDidPlayToEndTime,
    object: playerItem,
    queue: .main
) { _ in
    player.seek(to: .zero)
    player.play()
}
Accessibility

The animated intro has significant motion and timed fades; while reduce motion is handled, ensure VoiceOver focus order and accessibility announcements for appearing content (welcome text, button) are provided, and the video background is marked as decorative.

@State private var showVideoBackground: Bool = false
@State private var backgroundOpacity: Double = 1.0
@State private var centeredGradientOpacity: Double = 0.0
@State private var topGradientOpacity: Double = 0.0
@State private var bodyGradientOpacity: Double = 0.0
@State private var theme = WelcomeViewTheme()
@State private var isVideoReady = false
@State private var animationTask: Task<Void, Never>?

private let reduceMotionEnabled = UIAccessibility.isReduceMotionEnabled

let windowUUID: WindowUUID
let onFinish: () -> Void

enum AnimationPhase {
    case initial
    case phase1Complete
    case phase2Complete
    case phase3Complete
    case final
}

var body: some View {
    ZStack(alignment: .center) {
        // Matching Launch Screen background
        Color(.systemBackground)
            .ignoresSafeArea()
            .opacity(backgroundOpacity)

        // Video background (clipped to transition mask)
        ZStack {
            LoopingVideoPlayer(videoName: "welcome_background") {
                isVideoReady = true
            }

            // Centered radial gradient behind logo
            RadialGradient(
                gradient: Gradient(colors: [
                    Color.black.opacity(0.55),
                    Color.black.opacity(0)
                ]),
                center: .center,
                startRadius: 0,
                endRadius: UX.centeredGradientSize / 2
            )
            .opacity(centeredGradientOpacity)
        }
        .mask(
            RoundedRectangle(cornerRadius: UX.maskCornerRadius)
                .frame(height: transitionMaskHeight)
                .frame(maxWidth: transitionMaskWidth)
                .scaleEffect(transitionMaskScale, anchor: .center)
        )
        .ignoresSafeArea(edges: .all)
        .opacity(showVideoBackground ? 1 : 0)

        // Top vertical gradient behind logo
        if animationPhase == .phase3Complete {
            VStack(spacing: 0) {
                LinearGradient(
                    gradient: Gradient(colors: [
                        Color.black.opacity(0.38),
                        Color.black.opacity(0)
                    ]),
                    startPoint: .top,
                    endPoint: .bottom
                )
                .frame(height: topGradientHeight)

                Spacer()
            }
            .ignoresSafeArea()
            .opacity(topGradientOpacity)
        }

        // Welcome text
        Text(verbatim: .localized(.welcomeTo))
            .font(.system(size: UX.welcomeTextFontSize, weight: .semibold))
            .foregroundColor(theme.contentTextColor)
            .multilineTextAlignment(.center)
            .opacity(welcomeTextOpacity)
            .offset(y: welcomeTextOffset)
            .frame(maxWidth: transitionMaskWidth)

        // Logo
        Image("ecosiaLogoLaunch")
            .renderingMode(.template)
            .resizable()
            .aspectRatio(contentMode: .fit)
            .foregroundColor(logoColor)
            .frame(width: UX.logoWidth, height: UX.logoHeight)
            .opacity(logoOpacity)
            .offset(y: logoOffset)
            .frame(maxWidth: transitionMaskWidth)
            .accessibilityLabel(String.localized(.ecosiaLogoAccessibilityLabel))
            .accessibilityIdentifier(AccessibilityIdentifiers.Ecosia.logo)

        // Content
        if animationPhase == .phase3Complete {
            VStack {
                Spacer()

                VStack(spacing: UX.bodyTitleBottomSpacing) {
                    Text(verbatim: .localized(.realChangeAtYourFingertips))
                        .font(.ecosiaFamilyBrand(size: .ecosia.font._6l))
                        .foregroundStyle(theme.contentTextColor)
                        .multilineTextAlignment(.center)

                    Text(verbatim: .localized(.joinMillionsPeople))
                        .font(.system(size: 17, weight: .semibold))
                        .foregroundStyle(theme.contentTextColor)
                        .multilineTextAlignment(.center)
                }
                .padding(.top, UX.bodyGradientTopOffset)
                .padding(.bottom, UX.bodySubtitleBottomSpacing)
                .background(
                    LinearGradient(
                        gradient: Gradient(stops: [
                            .init(color: Color.black.opacity(0), location: 0.0),
                            .init(color: Color.black.opacity(0.35), location: 0.3),
                            .init(color: Color.black.opacity(0.24), location: 0.6),
                            .init(color: Color.black.opacity(0), location: 1.0)
                        ]),
                        startPoint: .top,
                        endPoint: .bottom
                    )
                    .frame(width: screenWidth)
                    .opacity(bodyGradientOpacity)
                )

                Button(action: {
                    Analytics.shared.introWelcome(action: .click)
                    startExitAnimation()
                }) {
                    Text(verbatim: .localized(.getStarted))
                        .font(.body)
                        .foregroundColor(theme.buttonTextColor)
                        .frame(maxWidth: .infinity)
                        .frame(height: UX.buttonHeight)
                        .background(theme.buttonBackgroundColor)
                        .cornerRadius(UX.buttonCornerRadius)
                }
            }
            .padding(.horizontal, UX.contentPadding)
            .padding(.top, UX.contentPadding)
            .padding(.bottom, UX.contentPadding + safeAreaBottom)
            .frame(maxWidth: contentMaxWidth)
            .opacity(bodyOpacity)
            .offset(y: bodyOffset)
        }
UI State Coupling

The toolbar animation assumes header/bottomContainer frames and safe area are laid out; confirm this is called after layout and handles different device rotations/orientations to prevent jumpy animations.

func prepareToolbarsForWelcomeTransition() {
    // Hide toolbars initially to prevent flash
    header.alpha = 0
    bottomContainer.alpha = 0

    // Hide content and set background color to match NTP
    contentStackView.alpha = 0
    let theme = themeManager.getCurrentTheme(for: windowUUID)
    // TODO: Investigate colors behind toolbars before transition
    // TODO: Smooth out color transitions
    view.backgroundColor = theme.colors.ecosia.backgroundPrimaryDecorative
}

/// Animates the top and bottom toolbars sliding in from the edges
/// This is called after the welcome screen fades out
func animateToolbarsIn() {
    let margin: CGFloat = 20 // Additional margin to ensure views are fully hidden
    let topOffset = -(header.frame.height + view.safeAreaInsets.top + margin)
    let bottomOffset = bottomContainer.frame.height + view.safeAreaInsets.bottom + margin

    // Set initial off-screen positions and make visible
    header.alpha = 1.0
    bottomContainer.alpha = 1.0
    header.transform = CGAffineTransform(translationX: 0, y: topOffset)
    bottomContainer.transform = CGAffineTransform(translationX: 0, y: bottomOffset)

    // Animate to final positions
    UIView.animate(
        withDuration: 0.35,
        delay: 0,
        options: .curveEaseInOut,
        animations: { [weak self] in
            self?.header.transform = .identity
            self?.bottomContainer.transform = .identity
            self?.contentStackView.alpha = 1
        }
    )
}

@github-actions
Copy link

github-actions bot commented Nov 13, 2025

PR Code Suggestions ✨

Latest suggestions up to a2df0be
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent duplicate browser start

Avoid calling startBrowser(with:) twice if a BrowserCoordinator already exists.
Starting again can cause duplicate controllers or animation conflicts. Only start
the browser when absent, then perform the transition animations.

firefox-ios/Client/Coordinators/Scene/SceneCoordinator.swift [147-163]

 func didFinishLaunch(from coordinator: LaunchCoordinator) {
-    startBrowser(with: nil)
-
-    guard let browserCoordinator = childCoordinators.first(where: { $0 is BrowserCoordinator }) as? BrowserCoordinator else {
+    if let browserCoordinator = childCoordinators.first(where: { $0 is BrowserCoordinator }) as? BrowserCoordinator {
+        browserCoordinator.browserViewController.prepareToolbarsForWelcomeTransition()
+        router.dismiss(animated: true) {
+            browserCoordinator.browserViewController.animateToolbarsIn()
+        }
+        remove(child: coordinator)
+    } else {
+        startBrowser(with: nil)
         router.dismiss(animated: true)
         remove(child: coordinator)
-        return
     }
-
-    browserCoordinator.browserViewController.prepareToolbarsForWelcomeTransition()
-    router.dismiss(animated: true) {
-        browserCoordinator.browserViewController.animateToolbarsIn()
-    }
-    remove(child: coordinator)
 }
Suggestion importance[1-10]: 8

__

Why: Avoiding a second startBrowser when a BrowserCoordinator already exists prevents duplicate controllers and conflicting animations; this aligns with the new transition flow and has meaningful impact.

Medium
Fix fallback image sizing

Ensure the fallback image fills the view by setting its frame initially. Without an
initial frame, it can render at zero size until a layout pass, causing a blank
screen flash on older iOS versions. Also assign it to match the view’s bounds when
added.

firefox-ios/Client/Ecosia/UI/Onboarding/LoopingVideoPlayer.swift [33-44]

 guard let videoURL = Bundle.main.url(forResource: videoName, withExtension: "mov") else {
     // Fallback to static image if video not found
     let imageView = UIImageView(image: UIImage(named: "forest"))
     imageView.contentMode = .scaleAspectFill
     imageView.autoresizingMask = [.flexibleWidth, .flexibleHeight]
+    imageView.frame = view.bounds
     view.addSubview(imageView)
     // Trigger onReady immediately for fallback
     DispatchQueue.main.async {
         onReady?()
     }
     return view
 }
Suggestion importance[1-10]: 7

__

Why: Adding imageView.frame = view.bounds ensures the fallback image fills the container immediately, preventing a transient blank area; the change is correct and low-risk but a minor improvement.

Medium
General
Prevent duplicate analytics display

Guard against multiple analytics emissions on re-appearance. SwiftUI views can
re-trigger onAppear during navigation or state changes, causing duplicate .display
events. Track and send the display event only once.

firefox-ios/Client/Ecosia/UI/Onboarding/WelcomeView.swift [215-226]

 .onAppear {
-    Analytics.shared.introWelcome(action: .display)
+    if animationPhase == .initial {
+        Analytics.shared.introWelcome(action: .display)
+    }
 
     logoColor = theme.brandPrimaryColor
 
     if reduceMotionEnabled {
         skipToFinalState()
     } else {
         // Animation will start when video is ready
     }
 }
Suggestion importance[1-10]: 6

__

Why: Guarding the .display event with animationPhase == .initial accurately reflects intent and reduces duplicate analytics; it’s reasonable but not critical and assumes animationPhase semantics.

Low

Previous suggestions

Suggestions up to commit 6f638a9
CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix invalid animation call

The trailing comma after the 'animations' closure causes a compiler error because no
completion parameter is provided. Remove the comma or add an explicit completion
block. This will fix the animation call and ensure toolbars animate in.

firefox-ios/Client/Ecosia/UI/Onboarding/BrowserViewController+WelcomeTransition.swift [39-49]

 UIView.animate(
     withDuration: 0.35,
     delay: 0,
     options: .curveEaseInOut,
     animations: { [weak self] in
         self?.header.transform = .identity
         self?.bottomContainer.transform = .identity
         self?.contentStackView.alpha = 1
-    },
+    }
 )
Suggestion importance[1-10]: 9

__

Why: Correctly identifies a trailing comma after the animations closure in UIView.animate which will not compile without a completion parameter; removing it matches UIKit API and prevents a runtime/compile error in the new code.

High
Ensure custom modal transition applies

Setting 'transitioningDelegate' on a UINavigationController has no effect unless
'modalPresentationStyle' is set to '.custom'. Explicitly set it to ensure the fade
transition is used on presentation/dismissal. This prevents default transitions from
showing and mismatch with toolbar animations.

firefox-ios/Client/Ecosia/UI/Onboarding/WelcomeNavigation.swift [12-25]

 init(rootViewController: UIViewController, windowUUID: WindowUUID) {
     self.windowUUID = windowUUID
 
-    // Transition delegate for fade dismissal
     let transition = FadeTransitionDelegate()
     let themeManager: ThemeManager = AppContainer.shared.resolve()
     let theme = themeManager.getCurrentTheme(for: windowUUID)
-    // Matches NTP background
     transition.dismissalBackgroundColor = theme.colors.ecosia.backgroundPrimaryDecorative
     self.fadeTransitionDelegate = transition
 
     super.init(rootViewController: rootViewController)
+    modalPresentationStyle = .custom
     transitioningDelegate = fadeTransitionDelegate
 }
Suggestion importance[1-10]: 7

__

Why: Setting modalPresentationStyle = .custom on the presenting controller is typically required for a custom transitioning delegate to take effect; adding it improves reliability of the intended fade transition, though behavior can vary based on presentation context.

Medium
General
Use GeometryReader for safe insets

Accessing 'UIApplication.shared.connectedScenes.first?.windows.first' can return
incorrect insets or nil, especially in multi‑window or offscreen contexts. Use the
view's safe area via SwiftUI GeometryReader to compute insets reliably. This
prevents mispositioned animations on iPad or during transitions.

firefox-ios/Client/Ecosia/UI/Onboarding/WelcomeView.swift [318-333]

-private var safeAreaTop: CGFloat {
-    if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
-       let window = windowScene.windows.first {
-        return window.safeAreaInsets.top
+@State private var safeAreaInsets: EdgeInsets = .init()
+
+var body: some View {
+    GeometryReader { geo in
+        ZStack(alignment: .center) {
+            // ... existing content ...
+        }
+        .onAppear {
+            safeAreaInsets = geo.safeAreaInsets
+        }
+        .onChange(of: geo.safeAreaInsets) { newInsets in
+            safeAreaInsets = newInsets
+        }
     }
-    return 0
+    .ignoresSafeArea()
+    .ecosiaThemed(windowUUID, $theme)
+    .onAppear {
+        Analytics.shared.introWelcome(action: .display)
+        logoColor = theme.brandPrimaryColor
+        startAnimationSequence()
+    }
 }
 
-private var safeAreaBottom: CGFloat {
-    if let windowScene = UIApplication.shared.connectedScenes.first as? UIWindowScene,
-       let window = windowScene.windows.first {
-        return window.safeAreaInsets.bottom
-    }
-    return 0
-}
+private var safeAreaTop: CGFloat { safeAreaInsets.top }
+private var safeAreaBottom: CGFloat { safeAreaInsets.bottom }
Suggestion importance[1-10]: 6

__

Why: Proposes a more robust way to get safe area insets in SwiftUI using GeometryReader, improving correctness across multi-window contexts; however, it requires broader refactoring and is not strictly necessary for functionality.

Low

@lucaschifino lucaschifino marked this pull request as ready for review November 13, 2025 12:23
@github-actions
Copy link

Persistent review updated to latest commit a2df0be

@lucaschifino lucaschifino requested a review from a team November 13, 2025 14:27
@lucaschifino lucaschifino changed the title [MOB-3897] Product tour welcome screen [MOB-3897] Welcome screen for product tour Nov 14, 2025
Copy link
Member

@d4r1091 d4r1091 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good on my end. Was wondering if we'd move the new onboarding as part of the Ecosia framework.
We'd also make our own EcosiaLaunchCoordinator (maybe not as part of the framework this one) to minimise the impact on the LaunchCoordinator and reset it to its original state altogether.
Leaving it here and keen to get to the Leftovers! 👀


import Foundation

extension Task where Success == Never, Failure == Never {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we move this into our Ecosia framework?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants