[MOB-3582] AI Shortcut Access Point on NTP #932

d4r1091 · 2025-08-09T08:42:24Z

MOB-3582

Context

We want an entry point on NTP in order to let users access the AI Search with ease.

Approach

All SwiftUI iOS16+

Added AI Search button to NTP header
Built multi-purpose header cell with AI access point (updated name from the "Login" one of the Auth work)
Added twinkle icon and button styling
Implemented Unleash flag

Before merging

Checklist

I performed some relevant testing on a real device and/or simulator for both iPhone and iPad
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)

- Add EcosiaAISearchButton to Ecosia framework with twinkle icon - Create NTPAIActionsCell with SwiftUI integration - Add NTPAIActionsCellViewModel with proper delegate pattern - Extend URLProvider with ai.search URLs for staging/production - Integrate with HomepageSectionType enum (.aiActions case) - Wire navigation through SharedHomepageCellDelegate pattern - Position AI search button right-aligned in new NTP section - Follow Ecosia comment conventions for modified Firefox code BREAKING CHANGE: Adds new .aiActions case to HomepageSectionType enum Note: Files need to be added to Xcode project build target

- Rename NTPAIActionsCell → NTPMultiPurposeEcosiaHeader - Rename NTPAIActionsCellViewModel → NTPMultiPurposeEcosiaHeaderViewModel - Update HomepageSectionType case: .aiActions → .multiPurposeEcosiaHeader - Rename delegate protocol: NTPAIActionsCellDelegate → NTPMultiPurposeEcosiaHeaderDelegate - Update delegate method: aiActionsCellDidRequestAISearch → multiPurposeEcosiaHeaderDidRequestAISearch - Move files from AIActions/ to MultiPurposeHeader/ directory - Update all references and variable names throughout codebase This better reflects the component's purpose as a multi-purpose header that can contain various Ecosia-specific actions beyond just AI search.

- Fix 'self used before super.init' error in LegacyHomepageViewController - Pass nil delegate during HomepageViewModel initialization - Set multiPurposeEcosiaHeaderViewModel.delegate after super.init() - Make delegate property internal and settable in view model - Remove trailing whitespace for SwiftLint compliance Build now compiles successfully with no errors or warnings.

@published

…kView pattern - Add @published theme properties to NTPMultiPurposeEcosiaHeaderViewModel - Implement applyTheme() method using theme.colors.ecosia colors - Add .onReceive listener for .ThemeDidChange notification - Use viewModel theme colors instead of themeManager in SwiftUI view - Update setTheme() to delegate to applyTheme() for consistency - Follow exact pattern used in Ecosia's FeedbackView for theme handling The AI search button now properly responds to system theme changes and manual theme switching in the app.

- Add aiSearchMVP case to Toggle.Name enum in Unleash.Model.swift - Create AISearchMVPExperiment.swift with public access for cross-target usage - Update NTPMultiPurposeEcosiaHeaderViewModel to use feature flag for isEnabled - Feature flag controls visibility of entire NTPMultiPurposeEcosiaHeader cell - Raw string flag: ai2-67-ai-search-mvp

…d code cleanup - Add UnleashAISearchMVPSetting debug class for testing feature flag - Add AI Search MVP setting to debug settings list - Code cleanup: remove trailing whitespace and fix formatting - Add missing newlines at end of files for Swift style consistency

- Add new ai_search category in Analytics.Category - Add ntpShortcut label in Analytics.Label.AISearch - Implement aiSearchNTPButtonTapped() method for tracking - Replace TODO comment with actual analytics call in NTPMultiPurposeEcosiaHeaderViewModel - Track AI Search button taps from New Tab Page for user engagement metrics

github-actions · 2025-08-09T08:43:13Z

PR Reviewer Guide 🔍

(Review updated until commit `6585715`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Fallback Handling Returning an empty cell identifier for iOS < 16.0 may cause dequeuing issues at runtime if the section is ever included; ensure the section is excluded for unsupported OS versions or provide a safe fallback identifier and handler. case .multiPurposeEcosiaHeader: if #available(iOS 16.0, ) { return NTPMultiPurposeEcosiaHeader.cellIdentifier } else { return "" // Fallback for iOS < 16.0 } case .climateImpactCounter: return NTPSeedCounterCell.cellIdentifier URL Query Param* The production AI search URL forces a query parameter to enable the feature; confirm this is intended and won’t duplicate/override Unleash logic or leak experiment flags in user-shared URLs. /// AI search URLs for different environments public var aiSearch: URL { switch self { case .staging: return root.appendingPathComponent("ai-search") case .production: var components = URLComponents(url: root.appendingPathComponent("ai-search"), resolvingAgainstBaseURL: false)! components.queryItems = [URLQueryItem(name: "feature-ai2-67-ai-search-mvp", value: "enabled")] return components.url! } } Analytics Coverage Only a tap event is tracked for the AI shortcut; consider tracking visibility/impression to measure feature exposure and add OS/version context if needed. // MARK: AI Search MVP public func aiSearchNTPButtonTapped() { track(Structured(category: Category.aiSearch.rawValue, action: Action.click.rawValue) .label(Analytics.Label.AISearch.ntpShortcut.rawValue)) }

github-actions · 2025-08-09T08:43:54Z

PR Code Suggestions ✨

Latest suggestions up to 6585715
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove force unwraps for URLs Force unwrapping `URLComponents` and its `url` risks crashing if the URL is malformed. Safely construct the URL and provide a resilient fallback to `root` or log an error to prevent app termination. firefox-ios/Ecosia/Core/Environment/URLProvider.swift [167-175] public var aiSearch: URL { + let base = root.appendingPathComponent("ai-search") switch self { case .staging: - return root.appendingPathComponent("ai-search") + return base case .production: - var components = URLComponents(url: root.appendingPathComponent("ai-search"), resolvingAgainstBaseURL: false)! - components.queryItems = [URLQueryItem(name: "feature-ai2-67-ai-search-mvp", value: "enabled")] - return components.url! + var components = URLComponents(url: base, resolvingAgainstBaseURL: false) + components?.queryItems = [URLQueryItem(name: "feature-ai2-67-ai-search-mvp", value: "enabled")] + return components?.url ?? base } } Suggestion importance[1-10]: 7 __ Why: Replacing force unwraps with safe optionals prevents potential crashes from malformed URLs, improving robustness without altering behavior.	Medium
Possible issue	Avoid empty cell identifier fallback Returning an empty cell identifier can cause dequeuing failures and runtime crashes on iOS < 16. Ensure the section is never created on unsupported OS versions instead of returning an empty identifier. Gate the section type behind availability when building sections and cell types to avoid invalid identifiers. firefox-ios/Client/Ecosia/Frontend/Home/EcosiaHomepageSectionType.swift [25-31] case .multiPurposeEcosiaHeader: if #available(iOS 16.0, *) { return NTPMultiPurposeEcosiaHeader.cellIdentifier } else { - return "" // Fallback for iOS < 16.0 + assertionFailure("multiPurposeEcosiaHeader should not be used on iOS < 16.0") + return NTPLogoCell.cellIdentifier // Safe fallback to a valid identifier } Suggestion importance[1-10]: 6 __ Why: Correctly identifies a risky empty identifier that could cause dequeuing issues on iOS < 16; proposed fix is reasonable, though alternate fallbacks or gating elsewhere may already mitigate it.	Low
General	Guard against missing delegate If `delegate` is nil, the tap produces only analytics without navigation, leading to a broken UX. Guard for a delegate and no-op analytics-only flows, or assert/log when missing to ensure navigation always happens. firefox-ios/Client/Ecosia/UI/NTP/MultiPurposeHeader/NTPMultiPurposeEcosiaHeaderViewModel.swift [46-49] func openAISearch() { - delegate?.multiPurposeEcosiaHeaderDidRequestAISearch() + guard let delegate else { + DefaultLogger.shared.log("NTPMultiPurposeEcosiaHeaderViewModel", "Delegate missing on AI Search tap", level: .warning) + return + } + delegate.multiPurposeEcosiaHeaderDidRequestAISearch() Analytics.shared.aiSearchNTPButtonTapped() } Suggestion importance[1-10]: 5 __ Why: Valid concern that a nil `delegate` leads to no navigation; the guard and logging improve UX predictability, but impact is moderate since delegate is usually set.	Low

Previous suggestions

Suggestions up to commit 6585715

Category	Suggestion	Impact
Possible issue	Avoid empty cell identifier Returning an empty cell identifier on iOS < 16 will cause dequeueing failures or crashes at runtime. Provide a valid identifier for a safe fallback cell or exclude this section entirely on unsupported OS versions. firefox-ios/Client/Ecosia/Frontend/Home/EcosiaHomepageSectionType.swift [25-31] case .multiPurposeEcosiaHeader: if #available(iOS 16.0, *) { return NTPMultiPurposeEcosiaHeader.cellIdentifier } else { - return "" // Fallback for iOS < 16.0 + return NTPLibraryCell.cellIdentifier // safe fallback identifier } Suggestion importance[1-10]: 7 __ Why: Correctly flags that returning an empty `cellIdentifier` for `.multiPurposeEcosiaHeader` on iOS < 16 can cause dequeue issues; however, proposing `NTPLibraryCell.cellIdentifier` as a fallback may mis-map section-to-cell semantics and could introduce UI inconsistencies. Better to hide the section on older OS or provide a dedicated fallback.	Medium
Possible issue	Align registered cells with identifiers Ensure `cellIdentifier` mapping aligns with registered cell types on all OS versions. If the section exists on iOS < 16, registering no corresponding type can crash dequeue; either always include a fallback type or prevent the section from being presented. firefox-ios/Client/Ecosia/Frontend/Home/EcosiaHomepageSectionType.swift [42-59] static var cellTypes: [ReusableCell.Type] { - var types: [ReusableCell.Type] = [] - - if #available(iOS 16.0, ) { - types.append(NTPMultiPurposeEcosiaHeader.self) - } - - types.append(contentsOf: [ + var types: [ReusableCell.Type] = [ NTPSeedCounterCell.self, NTPLogoCell.self, TopSiteItemCell.self, EmptyTopSiteCell.self, NTPLibraryCell.self, NTPImpactCell.self, NTPNewsCell.self, NTPCustomizationCell.self - ]) + ] + + if #available(iOS 16.0, ) { + types.append(NTPMultiPurposeEcosiaHeader.self) + } return types } Suggestion importance[1-10]: 6 __ Why: The concern about registered cell types aligning with identifiers is valid and helps avoid runtime crashes; the proposed reordering is mostly stylistic since the current code already conditionally appends the new cell for iOS 16+. Impact is moderate and largely preventive.	Low
General	Stabilize AI URL construction Hard-suffixed path "ai-search" may break if the root contains a trailing slash or locale path segments; use URLComponents to build consistently for both environments and avoid force unwraps to prevent crashes. firefox-ios/Ecosia/Core/Environment/URLProvider.swift [167-176] public var aiSearch: URL { - switch self { - case .staging: - return root.appendingPathComponent("ai-search") - case .production: - var components = URLComponents(url: root.appendingPathComponent("ai-search"), resolvingAgainstBaseURL: false)! - components.queryItems = [URLQueryItem(name: "feature-ai2-67-ai-search-mvp", value: "enabled")] - return components.url! + var components = URLComponents(url: root.appendingPathComponent("ai-search"), resolvingAgainstBaseURL: false) + if self == .production { + components?.queryItems = [URLQueryItem(name: "feature-ai2-67-ai-search-mvp", value: "enabled")] } + return components?.url ?? root } Suggestion importance[1-10]: 5 __ Why: Removing force unwraps improves safety and using URLComponents consistently is reasonable; however, the fallback to `root` on construction failure changes behavior silently and the trailing-slash/path concern is minor since `appendingPathComponent` handles it. Impact is modest.	Low

lucaschifino

Looking great! Just some questions as I will be taking it over as mentioned on this ticket comment.

lucaschifino · 2025-08-14T12:13:18Z

firefox-ios/Client/Ecosia/Frontend/Home/EcosiaHomepageSectionType.swift

-        return [
+        var types: [ReusableCell.Type] = []
+
+        if #available(iOS 16.0, *) {


Did you check with Product if no problem not supporting iOS 15? I see no issue and to be honest will likely soon be the case for the FF base, just checking.

lucaschifino · 2025-08-14T12:16:13Z

firefox-ios/Client/Ecosia/UI/NTP/MultiPurposeHeader/NTPMultiPurposeEcosiaHeader.swift

+
+/// NTP header cell containing multiple Ecosia-specific actions like AI search
+@available(iOS 16.0, *)
+final class NTPMultiPurposeEcosiaHeader: UICollectionViewCell, ThemeApplicable, ReusableCell {


I understand the descriptiveness, but to be honest I think NTPHeader might be a good enough name and the rest just making it longer 😅 Any specific reason for the longer name? I see as a pattern we also don't include Ecosia in others, not sure if you experience any conflicts.

lucaschifino · 2025-08-14T12:28:42Z

firefox-ios/Client/Ecosia/UI/NTP/MultiPurposeHeader/NTPMultiPurposeEcosiaHeaderViewModel.swift

+    func applyTheme(theme: Theme) {
+        self.theme = theme
+        buttonBackgroundColor = Color(theme.colors.ecosia.backgroundElevation1)
+        buttonIconColor = Color(theme.colors.ecosia.textPrimary)
+    }


It feels a bit bad to me that this is on the view model, but I remember this was already the case for the latest feedback view implementation. Can you remind me why that is? Couldn't we leave handling colors as a responsibility of the view similar to how SeedCounterView is?

github-actions · 2025-08-15T07:59:48Z

Persistent review updated to latest commit 6585715

lucaschifino

I changed bases to the feature branch mob-3505-ai-access-points and approving to merge into there to continue other work separately.

@d4r1091 feel free to still jump into the discussions and sort out confusions when you are back 🙂

d4r1091 added 10 commits August 6, 2025 15:26

[MOB-3582] Fix: no need to pass nil value

60a4f52

[MOB-3582] Update on the URLProvider aiSearch access

a727439

[MOB-3582] Swiftlint fixes

6585715

d4r1091 requested a review from a team August 9, 2025 08:42

d4r1091 marked this pull request as draft August 9, 2025 08:42

lucaschifino reviewed Aug 14, 2025

View reviewed changes

lucaschifino changed the base branch from main to mob-3505-ai-access-points August 15, 2025 07:58

lucaschifino marked this pull request as ready for review August 15, 2025 07:59

lucaschifino approved these changes Aug 15, 2025

View reviewed changes

lucaschifino merged commit 306ebe2 into mob-3505-ai-access-points Aug 15, 2025
4 of 5 checks passed

lucaschifino deleted the dc-mob-3582-ai-access-point-header branch August 15, 2025 08:02

This was referenced Aug 15, 2025

[MOB-3583] AI shortcut on native autocomplete #933

Merged

[MOB-3505] AI Search Access Points #934

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MOB-3582] AI Shortcut Access Point on NTP #932

[MOB-3582] AI Shortcut Access Point on NTP #932

Uh oh!

d4r1091 commented Aug 9, 2025 •

edited by jira bot

Loading

Uh oh!

github-actions bot commented Aug 9, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 9, 2025 •

edited

Loading

Uh oh!

lucaschifino left a comment

Uh oh!

lucaschifino Aug 14, 2025

Uh oh!

lucaschifino Aug 14, 2025

Uh oh!

lucaschifino Aug 14, 2025

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

lucaschifino left a comment

Uh oh!

Uh oh!

Uh oh!

[MOB-3582] AI Shortcut Access Point on NTP #932

[MOB-3582] AI Shortcut Access Point on NTP #932

Uh oh!

Conversation

d4r1091 commented Aug 9, 2025 • edited by jira bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Approach

Before merging

Checklist

Uh oh!

github-actions bot commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 6585715)

Uh oh!

github-actions bot commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

lucaschifino left a comment

Choose a reason for hiding this comment

Uh oh!

lucaschifino Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

lucaschifino Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

lucaschifino Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

lucaschifino left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

d4r1091 commented Aug 9, 2025 •

edited by jira bot

Loading

github-actions bot commented Aug 9, 2025 •

edited

Loading

(Review updated until commit `6585715`)

github-actions bot commented Aug 9, 2025 •

edited

Loading